Skip to content

Fix npm search routing and abbreviated packuments under cooldown - #567

Merged
andrew merged 1 commit into
git-pkgs:mainfrom
simonchrz:fix/npm-search-and-abbreviated
Oct 23, 2026
Merged

andrew merged 1 commit into
git-pkgs:mainfrom
simonchrz:fix/npm-search-and-abbreviated

Conversation

@simonchrz

Copy link
Copy Markdown
Contributor

Two npm bugs.

npm search returned other searches' results. /-/v1/search?text=... has no /-/ once the leading slash is trimmed, so it fell through to the package metadata handler. Upstream got /-/v1%2Fsearch without the query string, and the answer was cached under -/v1/search, so later searches were served the first one's results. /-/ping lost its query string the same way. Both are now relayed to upstream uncached with their query strings, like the signing-keys endpoint. /-/whoami and the user endpoints are left alone: the proxy sends its own upstream credentials, so relaying them would report the proxy's identity.

Cooldown filtered nothing when an abbreviated packument was still cached. Packuments share one metadata cache row whatever format they were fetched in. Without cooldown the proxy asks for the abbreviated format, which has no time map. After cooldown was turned on, a still-fresh abbreviated packument kept being served from the cache. It was also used as the stale copy when upstream failed. In both cases cooldown filtered nothing and the tarball check let recent versions through.

When full metadata is required (cooldown or npm_full_metadata), packument lookups now pass a validator that rejects an abbreviated document. It recognises one by a top-level modified and no time. The rejected document counts as a cache miss, is fetched again in full, and is never used as a stale fallback. A full document without time, as some registries serve, still passes. fetchMetadataDocument takes the validate callback that fetchOrCacheMetadata already has; the other callers pass nil.

Tests:

  • TestNPMSearchRelaysQueryUncached and TestNPMPingRelaysUpstream.
  • TestNPMCooldownRefetchesCachedAbbreviatedPackument and TestNPMCooldownTarballRefetchesCachedAbbreviatedPackument.
  • TestNPMCooldownSkipsAbbreviatedStaleFallback.

All five fail on main.

`npm search` requests /-/v1/search?text=... . The path has no "/-/"
after the leading slash is trimmed, so it fell through to the package
metadata handler: upstream got /-/v1%2Fsearch without the query
string, and the answer was cached under "-/v1/search", so every later
search was served the first one's results. Search and /-/ping (used
by `npm ping`, which lost its query string the same way) are now
relayed to upstream uncached with their query strings, like the
signing-keys endpoint.

Packuments share one metadata cache row whatever format they were
fetched in. Without cooldown the proxy asks for the abbreviated
format, which has no "time" map. After cooldown was turned on, a
still-fresh abbreviated packument kept being served from cache, and
used as the stale copy when upstream failed, so cooldown filtered
nothing and the tarball check let recent versions through. When full
metadata is required (cooldown or npm_full_metadata on), packument
lookups now pass a validator that rejects an abbreviated document
(top-level "modified", no "time"), so it counts as a cache miss, is
refetched in full and is never used as a stale fallback. Behaviour
without cooldown is unchanged.

fetchMetadataDocument takes the validate callback that
fetchOrCacheMetadata already has; other callers pass nil.
@andrew
andrew merged commit e050fbf into git-pkgs:main Oct 23, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants