Walkthrough

OSV vulnerability screening is extended from the resolver surface to all registry-serving endpoints (packument, tarball, dist-tags, version manifest). A new OsvVulnerability error variant is added with 403 Forbidden status, the OsvIndex gains an is_vulnerable query method, two helper functions implement packument filtering and tarball blocking, and the OSV index is now loaded whenever either the resolver or registry surface is enabled. The validate_tarball_name method is removed and its call sites replaced with parse_tarball_name.

Changes

OSV screening extended to registry serving path

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant serve_packument
  participant filter_osv_vulnerable_versions
  participant serve_tarball
  participant ensure_osv_allowed
  participant Upstream

  rect rgba(100, 149, 237, 0.5)
    note over Client,filter_osv_vulnerable_versions: Packument path
    Client->>serve_packument: GET /:package
    serve_packument->>Upstream: fetch packument
    Upstream-->>serve_packument: full packument JSON
    serve_packument->>filter_osv_vulnerable_versions: packument + osv_index
    filter_osv_vulnerable_versions-->>serve_packument: vulnerable versions/dist-tags/time removed
    serve_packument-->>Client: filtered packument (200)
  end

  rect rgba(220, 100, 100, 0.5)
    note over Client,ensure_osv_allowed: Tarball path
    Client->>serve_tarball: GET /:package/-/:filename
    serve_tarball->>ensure_osv_allowed: name + parsed version
    ensure_osv_allowed-->>serve_tarball: OsvVulnerability error
    serve_tarball-->>Client: 403 Forbidden (no upstream hit)
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Assessment against linked issues

Possibly related PRs

• pnpm/pnpm#12506: Introduces package version rejection/guard plumbing in the resolution path, which this PR complements by extending the same enforcement to the registry serving path.
• pnpm/pnpm#12563: Changes load_active_osv_index and OSV surface gating in server.rs; this PR's expansion of OSV loading to registry.enabled directly continues that work.

• X
• Mastodon
• Reddit
• LinkedIn

• Load the local OSV index for registry endpoints when osv.enabled is set.
• Filter packument, manifest, and dist-tag responses to omit vulnerable versions.
• Refuse vulnerable tarball downloads after access checks, before cache/upstream reads.

graph TD
  C(["npm client"]) --> R["pnpr registry routes"] --> A{"access allowed?"}
  A -->|"no"| AD["401/403"]
  A -->|"yes"| O{"OSV allows?"}
  O -->|"no"| OD["403 (tarball) / 404 (manifest)"]
  O -->|"yes"| K[("cache storage")] --> U[["uplink registry"]] --> S["served response"]

  R --> I[("OSV index")]

  subgraph Legend
    direction LR
    _cli(["Client"]) ~~~ _svc["Handler/service"] ~~~ _dec{"Decision"} ~~~ _db[("Data store")] ~~~ _ext[["External"]]
  end

Loading

The following are alternative approaches to this PR:

1. Filter at cache-write time (store sanitized packuments)

• ➕ Less per-request JSON parsing/filtering work after the first fetch
• ➕ Makes cached bytes match what clients will see
• ➖ Mutates upstream-cached bytes, complicating revalidation/debugging and cache correctness
• ➖ Harder to change policy later without rewarming cache

2. Enforce OSV only in resolver endpoints (status quo)

• ➕ Smaller runtime footprint on the registry surface
• ➕ Avoids JSON surgery on served packuments
• ➖ Registry users can still fetch vulnerable versions directly via packuments/tarballs
• ➖ Does not close the reported gap for registry-serving mode

3. Use live OSV API lookups instead of local DB

• ➕ Always freshest advisory data without local dumps
• ➖ Adds external dependency/latency and availability risk
• ➖ Harder to run air-gapped; higher operational complexity

Recommendation: The PR’s serve-time filtering/denial approach is the best trade-off: it closes the registry gap without mutating cached upstream bytes, keeps behavior consistent across proxied and cached responses, and (critically) preserves the existing private-package access boundary by running auth checks before OSV denial on tarballs. The added tests cover the key invariants (proxy vs cache, manifests, dist-tags, and tarball denial ordering).

Enhancement (1) +15 / -0

error.rsAdd explicit OSV vulnerability registry error mapped to 403 +15/-0

Add explicit OSV vulnerability registry error mapped to 403

• Introduces a new 'RegistryError::OsvVulnerability' variant with a user-facing message including advisory IDs. Registers the new error for logging kind and maps it to HTTP 403.

pnpr/crates/pnpr/src/error.rs

Bug fix (1) +81 / -13

server.rsApply OSV screening to registry responses and tarball downloads +81/-13

Apply OSV screening to registry responses and tarball downloads

• Loads the OSV index when either resolver or registry surfaces are enabled. Filters packuments, version-manifest reads, and dist-tag responses to omit vulnerable versions; removes dist-tags/time entries that point to filtered versions. For tarballs, parses the version from the filename, enforces access first, then returns 403 for OSV-blocked versions before reading from cache or fetching upstream.

pnpr/crates/pnpr/src/server.rs

Refactor (1) +0 / -13

package_name.rsUse tarball-name parsing that also returns version; drop validate helper +0/-13

Use tarball-name parsing that also returns version; drop validate helper

• Removes the standalone tarball-name validation helper and relies on 'parse_tarball_name' to both validate and extract the version string. This enables downstream logic (e.g., OSV screening) to act on the requested tarball version.

pnpr/crates/pnpr/src/package_name.rs

Tests (2) +206 / -6

tests.rsUpdate tarball-name tests to use version-extracting parser +5/-5

Update tarball-name tests to use version-extracting parser

• Adjusts tests to call 'parse_tarball_name' instead of the removed 'validate_tarball_name', preserving coverage of accepted and rejected filename forms.

pnpr/crates/pnpr/src/package_name/tests.rs

server.rsAdd integration tests for OSV filtering across proxy/cache and tarballs +201/-1

Add integration tests for OSV filtering across proxy/cache and tarballs

• Adds helpers to construct a minimal local OSV database and enable OSV in test configs. Introduces tests covering filtered packuments (proxy and cache), 404 for direct manifest of filtered versions, dist-tag filtering, tarball denial before upstream fetch, tarball denial from cache, and ensuring auth gating still returns 401 for private packages before OSV details are exposed.

pnpr/crates/pnpr/tests/server.rs

Documentation (1) +2 / -2

config.rsClarify OSV applies to all mounted surfaces, not just resolver +2/-2

Clarify OSV applies to all mounted surfaces, not just resolver

• Updates the OSV config docstring to reflect that the local OSV database can be consulted by any mounted surface (resolver and registry).

pnpr/crates/pnpr/src/config.rs

Description

filter_osv_vulnerable_versions() scans every packument version and calls OSV checks even when the
package has no advisories in the local OSV index, adding avoidable O(number_of_versions) CPU on the
hot packument GET path when osv.enabled is set. Because the OSV index only contains entries for
packages with advisories, a single package-level presence check can skip all filtering work for most
packages.

Code

pnpr/crates/pnpr/src/server.rs[R2119-2130]

+    let Some(osv_index) = osv_index else { return };
+    let package_name = name.as_str();
+    let mut blocked_keys = HashSet::new();
+    let mut retained_version_keys = HashSet::new();
+    let has_time = packument.get("time").and_then(Value::as_object).is_some();
+    if let Some(versions) = packument.get_mut("versions").and_then(Value::as_object_mut) {
+        versions.retain(|key, manifest| {
+            let manifest_version = manifest.get("version").and_then(Value::as_str);
+            let key_is_vulnerable = osv_index.is_vulnerable(package_name, key);
+            let manifest_is_vulnerable = manifest_version.is_some_and(|version| {
+                version != key && osv_index.is_vulnerable(package_name, version)
+            });

Evidence

The new packument filtering runs OSV checks per version entry; for packages absent from the OSV
index this work is provably unnecessary because the index map only contains packages that have
advisories (and thus missing means “no advisories”).

pnpr/crates/pnpr/src/server.rs[2114-2147]
pnpr/crates/pnpr/src/resolver/osv.rs[109-115]
pnpr/crates/pnpr/src/resolver/osv.rs[451-474]

Agent prompt

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

## Issue description
When `osv.enabled` is on, `filter_osv_vulnerable_versions()` currently iterates the packument and runs `osv_index.is_vulnerable(...)` per version entry even for packages that have **no** OSV advisories. Since OSV lookups are only meaningful for packages present in the OSV index, this adds avoidable work to a hot registry read path.
## Issue Context
`OsvIndex` only stores packages that appear in the OSV dump (i.e., packages with advisories). Most packages will not be present, so the common case should short-circuit before scanning `versions`, `dist-tags`, and `time`.
## Fix Focus Areas
- pnpr/crates/pnpr/src/resolver/osv.rs[109-137]
- pnpr/crates/pnpr/src/resolver/osv.rs[478-488]
- pnpr/crates/pnpr/src/server.rs[2114-2157]
### Suggested implementation shape
- Add a cheap method on `OsvIndex`, e.g. `pub(crate) fn has_advisories(&self, name: &str) -> bool`, implemented as `self.packages.contains_key(normalized_name(name).as_ref())`.
- In `filter_osv_vulnerable_versions()`, early-return if `!osv_index.has_advisories(package_name)`.
- Optionally apply the same early-return pattern in `filter_osv_vulnerable_dist_tags()` / `is_osv_vulnerable_packument_version()` / `ensure_osv_allowed()` to avoid unnecessary work on those endpoints too.

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

Description

In filter_osv_vulnerable_versions(), OSV vulnerability is checked for every version key to build
blocked_keys, then checked again for each entry in the packument's time object, redoing the same
advisory scan for all safe versions. On large packuments, this adds avoidable CPU and tail latency
on the hot packument GET path when OSV is enabled.

Code

pnpr/crates/pnpr/src/server.rs[R2143-2146]

+    if let Some(time) = packument.get_mut("time").and_then(Value::as_object_mut) {
+        time.retain(|key, _| {
+            !blocked_keys.contains(key) && !osv_index.is_vulnerable(package_name, key)
+        });

Evidence

The function already calls osv_index.is_vulnerable() while iterating versions to build
blocked_keys, then calls osv_index.is_vulnerable() again inside time.retain(...) for each
time key not in blocked_keys, which includes all safe version keys. This duplicates OSV advisory
scans for the common case where time contains all version keys.

pnpr/crates/pnpr/src/server.rs[2119-2135]
pnpr/crates/pnpr/src/server.rs[2143-2147]
pnpr/crates/pnpr/src/resolver/osv.rs[109-115]

Agent prompt

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

## Issue description
`filter_osv_vulnerable_versions()` performs OSV checks while building `blocked_keys` from the `versions` map, and then performs OSV checks again when filtering the `time` map. For the typical case where `time` keys mirror `versions` keys, this re-runs expensive OSV advisory scans for every non-blocked version.
### Issue Context
This runs on the hot registry packument serving path (`GET /:pkg`). With OSV enabled, packuments with many versions will pay this extra work per request.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2119-2147]
### Suggested fix
When `versions` is present, treat `blocked_keys` as the authoritative set of version keys to remove and avoid re-calling `osv_index.is_vulnerable()` for `time` keys.
Example approach:
- Keep `modified` (and `created` if present) unconditionally.
- For other `time` keys, remove if `blocked_keys.contains(key)`; otherwise keep.
If you still want to be defensive for malformed packuments where `time` may contain version keys not in `versions`, you can limit OSV checks to only those `time` keys that are not known special keys and not present in the `versions` object (requires capturing the post-filter `versions` keys or a flag indicating whether `versions` existed).

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

Description

filter_osv_vulnerable_versions() does a full versions-map iteration to build blocked_keys and then
iterates the same map again via retain(), adding avoidable O(number_of_versions) work per packument
response when OSV is enabled. On large packuments this increases CPU and tail latency on the hot
registry GET path.

Code

pnpr/crates/pnpr/src/server.rs[R2117-2131]

+    let mut blocked_keys = HashSet::new();
+    if let Some(versions) = packument.get_mut("versions").and_then(Value::as_object_mut) {
+        blocked_keys = versions
+            .iter()
+            .filter_map(|(key, manifest)| {
+                let manifest_version = manifest.get("version").and_then(Value::as_str);
+                let key_is_vulnerable = osv_index.is_vulnerable(package_name, key);
+                let manifest_is_vulnerable = manifest_version.is_some_and(|version| {
+                    version != key && osv_index.is_vulnerable(package_name, version)
+                });
+                (key_is_vulnerable || manifest_is_vulnerable).then(|| key.clone())
+            })
+            .collect();
+        versions.retain(|key, _| !blocked_keys.contains(key));
+    }

Evidence

packument_response() calls filter_osv_vulnerable_versions() on every packument response when OSV is
enabled, and the filter implementation iterates versions twice (first with .iter() +
.collect(), then again with versions.retain(...)).

pnpr/crates/pnpr/src/server.rs[2091-2107]
pnpr/crates/pnpr/src/server.rs[2110-2131]

Agent prompt

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

## Issue description
`filter_osv_vulnerable_versions()` currently performs two passes over the `versions` object: one to collect `blocked_keys`, then another via `versions.retain(...)`. This adds avoidable CPU work and hash lookups on every packument response when OSV is enabled.
### Issue Context
Packument serving is a hot read path. The OSV filter is called from `packument_response()` for every packument request (including cached packuments), so the extra traversal scales with packument size.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2110-2144]
### Suggested fix approach
- Convert the `versions` filtering to a single `versions.retain(|key, manifest| { ... })` pass.
- Inside the retain closure, compute whether the entry is blocked.
- If blocked, `blocked_keys.insert(key.clone())` and return `false`.
- Otherwise return `true`.
- Keep the subsequent `dist-tags` / `time` filtering using `blocked_keys` (and only fall back to additional OSV lookups if strictly needed).

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

Description

filter_osv_vulnerable_versions() calls is_vulnerable() for both the versions map key and the
nested manifest's version field, which is redundant when they match (the common case). For large
packuments, this approximately doubles OSV parsing/advisory scans per version entry.

Code

pnpr/crates/pnpr/src/server.rs[R2118-2125]

+        blocked_keys = versions
+            .iter()
+            .filter_map(|(key, manifest)| {
+                let manifest_version = manifest.get("version").and_then(Value::as_str);
+                (osv_index.is_vulnerable(package_name, key)
+                    || manifest_version
+                        .is_some_and(|version| osv_index.is_vulnerable(package_name, version)))
+                .then(|| key.clone())

Evidence

The new filter calls is_vulnerable() twice per version entry; is_vulnerable() parses the version and
scans advisories, so repeating it for identical strings is wasted work.

pnpr/crates/pnpr/src/server.rs[2109-2142]
pnpr/crates/pnpr/src/resolver/osv.rs[109-115]

Agent prompt

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

## Issue description
`filter_osv_vulnerable_versions()` checks both `key` and `manifest["version"]` against OSV. Most upstream packuments have `manifest.version == key`, so the second lookup typically repeats the same work.
### Issue Context
The second check is useful for guarding against identity mismatches, but it can be made conditional to preserve that protection without double work.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2109-2142]
- pnpr/crates/pnpr/src/resolver/osv.rs[109-115]
### Suggested change
Change the per-entry logic to:
- Compute `let key_vuln = osv_index.is_vulnerable(package_name, key);`
- Only if `manifest_version` exists and `manifest_version != key`, compute the second `is_vulnerable` check.
This keeps the mismatch defense while removing redundant OSV scans for well-formed packuments.

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