Fix npm search routing and abbreviated packuments under cooldown

README.md

@@ -1207,6 +1207,7 @@ Recently cached:
 | `GET /npm/*` | npm registry protocol |
 | `POST /npm/-/npm/v1/security/*` | npm/pnpm/Yarn audit endpoints, passed through to upstream |
 | `GET /npm/-/npm/v1/keys` | npm registry signing keys, passed through to upstream |
+| `GET /npm/-/v1/search`, `GET /npm/-/ping` | `npm search` and `npm ping`, passed through to upstream uncached |
 | `GET /cargo/*` | Cargo sparse index protocol |
 | `GET /gem/*` | RubyGems protocol |
 | `GET /go/*` | Go module proxy protocol |

docs/configuration.md

@@ -353,7 +353,7 @@ upstream:
 - Composer metadata of routed packages carries an empty `notification-url`, so Composer does not report their installs to the default upstream's `notify-batch` endpoint.
 - Use `upstream.auth` for the registry's credentials. The proxy has no authentication of its own, so every client that can reach it can download routed packages.
 - Search (`/composer/search.json`) and the package list (`/composer/packages/list.json`) still come from the default upstream only.
-- npm audit and signing-key requests still go to `upstream.npm`. An audit request lists every package in the dependency tree, routed ones included.
+- npm audit, signing-key, search and ping requests still go to `upstream.npm`. An audit request lists every package in the dependency tree, routed ones included.
 - Routes can only be set in the configuration file, not with environment variables.
 
 Helm HTTP repositories and additional OCI registries are configured as named maps:
@@ -982,6 +982,8 @@ Resolution order: exact package override, then package pattern, then ecosystem o
 
 Currently supported for npm, PyPI, pub.dev, Composer, Cargo, NuGet, Conda, RubyGems, and Hex. These ecosystems include publish timestamps in their metadata.
 
+With cooldown on, the proxy asks npm for full packuments, since the abbreviated ones it uses otherwise carry no publish times. Abbreviated packuments cached before cooldown was turned on are fetched again in full rather than served, including as a stale copy when the upstream is down.
+
 Note: Hex cooldown requires disabling registry signature verification since the proxy re-encodes the protobuf payload without the original signature. Set `HEX_NO_VERIFY_REPO_ORIGIN=1` or configure your repo with `no_verify: true`.
 
 ## Version denylist

internal/handler/chef.go

@@ -133,7 +133,7 @@ func (h *ChefHandler) serveDocument(w http.ResponseWriter, r *http.Request, docu
 		return
 	}
 
-	upstream := h.proxy.fetchMetadataDocument(r.Context(), chefEcosystem, cacheKey, upstreamURL, contentTypeJSON)
+	upstream := h.proxy.fetchMetadataDocument(r.Context(), chefEcosystem, cacheKey, upstreamURL, contentTypeJSON, nil)
 	if err := upstream.err; err != nil {
 		if errors.Is(err, ErrUpstreamNotFound) {
 			http.Error(w, "not found", http.StatusNotFound)

internal/handler/composer.go

@@ -169,7 +169,7 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R
 		return
 	}
 
-	upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON)
+	upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON, nil)
 	if err := upstream.err; err != nil {
 		if errors.Is(err, ErrUpstreamNotFound) {
 			http.Error(w, "not found", http.StatusNotFound)
@@ -753,7 +753,7 @@ func (h *ComposerHandler) cachedDist(ctx context.Context, files []composerMetada
 		return composerDist{}, "", false
 	}
 	for _, f := range files {
-		doc := h.proxy.fetchMetadataDocument(ctx, "composer", f.cacheKey, f.url, contentTypeJSON)
+		doc := h.proxy.fetchMetadataDocument(ctx, "composer", f.cacheKey, f.url, contentTypeJSON, nil)
 		if doc.err != nil {
 			continue
 		}

internal/handler/composer_hosted.go

@@ -78,7 +78,7 @@ func pushedComposerVersion(local []composerLocalEntry, version string) bool {
 // upstream does not know the package, the document holds only the pushed
 // versions.
 func (h *ComposerHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http.Request, packageName, upstreamURL, cacheKey string, local []composerLocalEntry) {
-	upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON)
+	upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON, nil)
 	var out []byte
 	var err error
 	switch {

internal/handler/handler.go

@@ -1335,9 +1335,10 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u
 
 // fetchMetadataDocument is FetchOrCacheMetadata for callers that rewrite the
 // document and need to know which one they got: the result carries its
-// digest along with the body.
-func (p *Proxy) fetchMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string) metadataResult {
-	return p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, "", nil)
+// digest along with the body. validate, which may be nil, is as for
+// fetchOrCacheMetadata.
+func (p *Proxy) fetchMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func([]byte) error) metadataResult {
+	return p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, "", validate)
 }
 
 // lookupMetadata implements fetchOrCacheMetadata and fetchMetadataDocument.

internal/handler/metadata_not_modified_test.go

@@ -36,7 +36,7 @@ func TestMetadataNotModifiedKeepsStoredDocument(t *testing.T) {
 	proxy.HTTPClient = upstream.Client()
 
 	ctx := context.Background()
-	first := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON)
+	first := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil)
 	if first.err != nil {
 		t.Fatal(first.err)
 	}
@@ -46,7 +46,7 @@ func TestMetadataNotModifiedKeepsStoredDocument(t *testing.T) {
 	}
 
 	for range 2 {
-		res := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON)
+		res := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil)
 		if res.err != nil || string(res.body) != body || res.digest != first.digest {
 			t.Fatalf("revalidated fetch = %q, digest %q, err %v; want %q, digest %q", res.body, res.digest, res.err, body, first.digest)
 		}
@@ -99,7 +99,7 @@ func TestMetadataNotModifiedStoresRowWithoutDigest(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	res := proxy.fetchMetadataDocument(context.Background(), "test", "pkg", upstream.URL+"/pkg", contentTypeJSON)
+	res := proxy.fetchMetadataDocument(context.Background(), "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil)
 	if res.err != nil || string(res.body) != `{"v":1}` {
 		t.Fatalf("fetch = %q, %v", res.body, res.err)
 	}

internal/handler/npm.go

@@ -32,6 +32,11 @@ const (
 	// npmKeysPath serves the registry signing keys `npm audit signatures` reads.
 	npmKeysPath = "/-/npm/v1/keys"
 
+	// npmSearchPath is what `npm search` queries, with the terms in the query
+	// string. npmPingPath is what `npm ping` checks.
+	npmSearchPath = "/-/v1/search"
+	npmPingPath   = "/-/ping"
+
 	// npmSecurityMaxBody caps the audit payload. Buffering it lets an oversized
 	// body be refused before the upstream request starts.
 	npmSecurityMaxBody = 16 << 20
@@ -88,10 +93,21 @@ func (h *NPMHandler) Routes() http.Handler {
 			return
 		}
 
-		if r.URL.Path == npmKeysPath {
+		switch r.URL.Path {
+		case npmKeysPath:
 			h.proxy.ProxyUpstream(w, r, h.upstreamURL+npmKeysPath,
 				[]string{headerAccept, headerAcceptEncoding})
 			return
+		case npmSearchPath, npmPingPath:
+			// Every query has its own answer, so these are relayed uncached
+			// and keep their query string.
+			upstreamURL := h.upstreamURL + r.URL.Path
+			if r.URL.RawQuery != "" {
+				upstreamURL += "?" + r.URL.RawQuery
+			}
+			h.proxy.ProxyUpstream(w, r, upstreamURL,
+				[]string{headerAccept, headerAcceptEncoding})
+			return
 		}
 
 		path := strings.TrimPrefix(r.URL.Path, "/")
@@ -224,7 +240,11 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept)
+	var validate func([]byte) error
+	if !versionLookup {
+		validate = h.packumentValidator()
+	}
+	upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept, validate)
 	if err := upstream.err; err != nil {
 		if errors.Is(err, ErrUpstreamNotFound) {
 			if versionLookup {
@@ -292,11 +312,52 @@ func (h *NPMHandler) packumentAccept() string {
 	return npmAcceptDefault
 }
 
+// errAbbreviatedPackument rejects an abbreviated packument where the full one
+// is needed.
+var errAbbreviatedPackument = errors.New("abbreviated packument where full metadata is required")
+
+// packumentValidator returns the check a packument must pass to be served or
+// cached, or nil when any document will do. Packuments share one cache row
+// whatever format they were fetched in, so one cached in abbreviated form
+// before cooldown or full metadata was turned on would otherwise still be
+// served, and used for the cooldown tarball check, while it stayed fresh or
+// as a stale copy when upstream failed. It has no "time" map, so cooldown
+// would let every version through. Rejecting it makes the lookup fetch the
+// full document instead.
+func (h *NPMHandler) packumentValidator() func([]byte) error {
+	if h.packumentAccept() != contentTypeJSON {
+		return nil
+	}
+	return requireFullPackument
+}
+
+// requireFullPackument rejects an abbreviated packument: one with the
+// top-level "modified" field the abbreviated format carries and no "time"
+// map. A full document without a time map, as some registries serve, is
+// let through as before, and so is anything that is not a JSON object,
+// leaving it to the usual handling.
+func requireFullPackument(body []byte) error {
+	var hasTime, hasModified bool
+	err := forEachJSONMember(body, func(m jsonMember) error {
+		switch key := m.key(body); {
+		case jsonKeyIs(key, "time"):
+			hasTime = true
+		case jsonKeyIs(key, "modified"):
+			hasModified = true
+		}
+		return nil
+	})
+	if err == nil && hasModified && !hasTime {
+		return errAbbreviatedPackument
+	}
+	return nil
+}
+
 // handleFilteredNPMVersionMetadata serves /pkg/<version> and /pkg/<tag> from the
 // filtered packument so cooldown and dist-tags match the root metadata response.
 func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r *http.Request, packageName, specifier, accept, registryURL, cacheKey, metadataCacheKey string) {
 	_, packumentPath := npmMetadataPath(packageName)
-	packument, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", cacheKey, registryURL+"/"+packumentPath, accept)
+	packument, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), "npm", cacheKey, registryURL+"/"+packumentPath, "", h.packumentValidator(), accept)
 	if err != nil {
 		if errors.Is(err, ErrUpstreamNotFound) {
 			JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error())
@@ -859,7 +920,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) {
 	registryURL, cacheKey := h.sourceFor(packageName)
 	metadata := sync.OnceValues(func() ([]byte, error) {
 		upstreamURL := registryURL + "/" + url.PathEscape(packageName)
-		body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", cacheKey, upstreamURL, h.packumentAccept())
+		body, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), "npm", cacheKey, upstreamURL, "", h.packumentValidator(), h.packumentAccept())
 		return body, err
 	})
 	if h.versionInCooldown(packageName, version, metadata) {

internal/handler/npm_hosted.go

@@ -98,7 +98,7 @@ func (h *NPMHandler) serveLocalMetadata(w http.ResponseWriter, r *http.Request,
 // When the upstream does not know the package, the packument holds only the
 // pushed versions.
 func (h *NPMHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http.Request, packageName, upstreamURL, accept, metadataCacheKey string, local []npmLocalEntry) {
-	upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept)
+	upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept, h.packumentValidator())
 	var out []byte
 	var err error
 	switch {

internal/handler/npm_packument_accept_test.go

@@ -1,15 +1,18 @@
 package handler
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
+	"github.com/git-pkgs/cooldown"
 	"github.com/git-pkgs/registries/fetch"
 )
 
@@ -76,3 +79,132 @@ func TestNPMTarballKeepsCachedPackumentFormat(t *testing.T) {
 		t.Errorf("cached packument Content-Type = %q, want %q", entry.ContentType.String, abbreviated)
 	}
 }
+
+// abbreviatedThenFullRegistry serves a packument with a 2.0.0 published an
+// hour ago and a 1.0.0 published a month ago. The abbreviated form, like
+// registry.npmjs.org's, has no time map. fail makes requests for the full
+// document fail.
+func abbreviatedThenFullRegistry(t *testing.T, fail *atomic.Bool, fullRequests *atomic.Int64) *httptest.Server {
+	t.Helper()
+	const abbreviated = "application/vnd.npm.install-v1+json"
+	var upstream *httptest.Server
+	upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/demo" {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		versions := fmt.Sprintf(`{"1.0.0":{"name":"demo","version":"1.0.0","dist":{"tarball":%q}},"2.0.0":{"name":"demo","version":"2.0.0","dist":{"tarball":%q}}}`,
+			upstream.URL+"/demo/-/demo-1.0.0.tgz", upstream.URL+"/demo/-/demo-2.0.0.tgz")
+		if strings.HasPrefix(r.Header.Get("Accept"), abbreviated) {
+			w.Header().Set(headerContentType, abbreviated)
+			_, _ = fmt.Fprintf(w, `{"name":"demo","modified":%q,"dist-tags":{"latest":"2.0.0"},"versions":%s}`,
+				time.Now().Format(time.RFC3339), versions)
+			return
+		}
+		fullRequests.Add(1)
+		if fail.Load() {
+			w.WriteHeader(http.StatusServiceUnavailable)
+			return
+		}
+		w.Header().Set(headerContentType, contentTypeJSON)
+		_, _ = fmt.Fprintf(w, `{"name":"demo","dist-tags":{"latest":"2.0.0"},"time":{"1.0.0":%q,"2.0.0":%q},"versions":%s}`,
+			time.Now().Add(-30*24*time.Hour).Format(time.RFC3339), time.Now().Add(-time.Hour).Format(time.RFC3339), versions)
+	}))
+	t.Cleanup(upstream.Close)
+	return upstream
+}
+
+// seedAbbreviatedPackument caches the abbreviated packument for demo through
+// the metadata route while cooldown is off, then turns cooldown on.
+func seedAbbreviatedPackument(t *testing.T, upstream *httptest.Server) *NPMHandler {
+	t.Helper()
+	proxy, db, _, artifactFetcher := setupTestProxy(t)
+	proxy.CacheMetadata = true
+	proxy.MetadataTTL = time.Hour
+	proxy.HTTPClient = upstream.Client()
+	artifactFetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("package")), ContentType: "application/gzip"}
+	h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL)
+
+	if w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil); w.Code != http.StatusOK {
+		t.Fatalf("seeding metadata: status %d: %s", w.Code, w.Body.String())
+	}
+	entry, err := db.GetMetadataCache("npm", "demo")
+	if err != nil || entry == nil || entry.ContentType.String != "application/vnd.npm.install-v1+json" {
+		t.Fatalf("expected an abbreviated packument in the cache, got %+v (%v)", entry, err)
+	}
+
+	proxy.Cooldown = &cooldown.Config{Default: "7d"}
+	return h
+}
+
+// A packument cached in abbreviated form before cooldown was turned on has
+// no publish times, so cooldown could not filter anything while it stayed
+// fresh. With cooldown on it must be refetched in full.
+func TestNPMCooldownRefetchesCachedAbbreviatedPackument(t *testing.T) {
+	var fail atomic.Bool
+	var fullRequests atomic.Int64
+	upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests)
+	h := seedAbbreviatedPackument(t, upstream)
+
+	w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("metadata: status %d: %s", w.Code, w.Body.String())
+	}
+	var doc struct {
+		DistTags map[string]string          `json:"dist-tags"`
+		Versions map[string]json.RawMessage `json:"versions"`
+	}
+	if err := json.Unmarshal(w.Body.Bytes(), &doc); err != nil {
+		t.Fatalf("decoding metadata: %v", err)
+	}
+	if _, ok := doc.Versions["2.0.0"]; ok {
+		t.Error("version 2.0.0, published an hour ago, was not filtered by cooldown")
+	}
+	if _, ok := doc.Versions["1.0.0"]; !ok {
+		t.Error("version 1.0.0 missing")
+	}
+	if doc.DistTags["latest"] != "1.0.0" {
+		t.Errorf("dist-tags.latest = %q, want 1.0.0", doc.DistTags["latest"])
+	}
+	if fullRequests.Load() == 0 {
+		t.Error("full packument was never fetched from upstream")
+	}
+
+	if w := serveNPM(t, h, http.MethodGet, "/demo/-/demo-2.0.0.tgz", nil, nil); w.Code != http.StatusNotFound {
+		t.Errorf("tarball 2.0.0: status %d, want %d (withheld by cooldown)", w.Code, http.StatusNotFound)
+	}
+}
+
+// The tarball check alone must not trust the abbreviated packument either.
+func TestNPMCooldownTarballRefetchesCachedAbbreviatedPackument(t *testing.T) {
+	var fail atomic.Bool
+	var fullRequests atomic.Int64
+	upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests)
+	h := seedAbbreviatedPackument(t, upstream)
+
+	if w := serveNPM(t, h, http.MethodGet, "/demo/-/demo-2.0.0.tgz", nil, nil); w.Code != http.StatusNotFound {
+		t.Errorf("tarball 2.0.0: status %d, want %d (withheld by cooldown)", w.Code, http.StatusNotFound)
+	}
+	if fullRequests.Load() == 0 {
+		t.Error("full packument was never fetched from upstream")
+	}
+}
+
+// When upstream cannot supply the full packument, the abbreviated one must not
+// stand in for it as a stale copy: it would let every version through.
+func TestNPMCooldownSkipsAbbreviatedStaleFallback(t *testing.T) {
+	var fail atomic.Bool
+	var fullRequests atomic.Int64
+	upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests)
+	h := seedAbbreviatedPackument(t, upstream)
+	h.proxy.MetadataTTL = time.Nanosecond // force a refetch
+	fail.Store(true)
+
+	w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil)
+	if w.Code != http.StatusBadGateway {
+		t.Errorf("metadata: status %d, want %d; body: %s", w.Code, http.StatusBadGateway, w.Body.String())
+	}
+	if strings.Contains(w.Body.String(), "2.0.0") {
+		t.Errorf("served the abbreviated packument as a stale copy: %s", w.Body.String())
+	}
+}