From 09e41d0272c34e9433d22cf265e46995960041ba Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 24 Oct 2026 08:11:25 +0200 Subject: [PATCH 1/4] Fail closed in gem, Hex and PyPI cooldown RubyGems, Hex and PyPI keep publish times outside the metadata clients read, so cooldown fetches them from a second API. When that fetch failed, was rate limited, or returned something that didn't parse, the compact index, Hex package or simple page was served unfiltered, and a Hex package that couldn't be re-encoded was passed on as upstream sent it. A flaky or rate-limited timestamp API let every version in cooldown through. These now fail with 502, or with 503 and Retry-After when the rate limit gate held the request, as the Go handler already does. PyPI file downloads check the same way. A PyPI package the JSON API answers 404 for still has no times and is served unfiltered, as before. npm, Composer, pub.dev and PyPI JSON metadata that couldn't be rewritten was also passed on unfiltered unless the package had denied versions. It now fails with 502 whenever cooldown applies too. npm tarball downloads whose packument couldn't be fetched skipped the cooldown check and downloaded straight from upstream; they now fail. All of this only applies when cooldown covers the package: packages exempted with a duration of 0 skip the timestamp lookups, and the gem and Hex handlers serve them through the metadata cache as when cooldown is off. --- docs/configuration.md | 2 + internal/handler/composer.go | 5 + internal/handler/cooldown_fail_closed.go | 34 +++ internal/handler/cooldown_fail_closed_test.go | 271 ++++++++++++++++++ internal/handler/gem.go | 33 +-- internal/handler/gem_test.go | 12 +- internal/handler/hex.go | 39 ++- internal/handler/npm.go | 46 +-- internal/handler/npm_test.go | 4 +- internal/handler/pub.go | 5 + internal/handler/pypi.go | 65 +++-- 11 files changed, 424 insertions(+), 92 deletions(-) create mode 100644 internal/handler/cooldown_fail_closed.go create mode 100644 internal/handler/cooldown_fail_closed_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 49156494..4712d696 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1042,6 +1042,8 @@ Maven caveats: - Maven Central lists versions in version order, not in the order they were published. A patch to an older release line published after a newer release that is out of cooldown stays listed while it is in cooldown, but its files are still refused. - SNAPSHOT versions are not held back. +RubyGems, Hex and PyPI keep their publish times outside the metadata clients read: the RubyGems versions API, the Hex HTTP API (`upstream.hex_api`) and the PyPI JSON API. The proxy fetches them alongside each compact index, package or simple page it filters, and for PyPI downloads. Cooldown fails closed: when it applies to a package and those times can't be fetched or parsed, or the metadata can't be rewritten, the request fails with 502, or with 503 and `Retry-After` while the upstream is rate limiting the proxy, instead of serving the metadata unfiltered. The same goes for npm, Composer and pub.dev metadata that can't be rewritten, and for npm tarball downloads whose publish time can't be read. Packages exempted with a duration of `0` are served as before and don't need the times. A PyPI package the JSON API returns 404 for has no times to go by and is served unfiltered. + 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. Conda cooldown covers `repodata.json` and sharded repodata (CEP 16). Clients cache shards by the hash in the shard index, so with cooldown on the proxy serves a rewritten index whose shard hashes change every hour and filters each shard as it is fetched. Versions leaving cooldown show up within the hour, and clients fetch the shards they need again once an hour. If a channel's shard index points packages or shards somewhere the proxy doesn't route, the proxy answers 404 for it and clients fall back to `repodata.json`. diff --git a/internal/handler/composer.go b/internal/handler/composer.go index d0c59ffe..92eae0ae 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -188,6 +188,11 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R if r.Context().Err() != nil { return // the client left while waiting on a shared rewrite } + if h.proxy.mustFilterMetadata("composer", basePackage) { + h.proxy.Logger.Error("failed to filter composer metadata", "package", basePackage, "error", err) + http.Error(w, "failed to filter package metadata", http.StatusBadGateway) + return + } h.proxy.Logger.Warn("failed to rewrite metadata, proxying original", "error", err) markStale(w, upstream.stale) w.Header().Set(headerContentType, "application/json") diff --git a/internal/handler/cooldown_fail_closed.go b/internal/handler/cooldown_fail_closed.go new file mode 100644 index 00000000..3a0a7f44 --- /dev/null +++ b/internal/handler/cooldown_fail_closed.go @@ -0,0 +1,34 @@ +package handler + +import "net/http" + +// Metadata for a package that cooldown or the denylist applies to is never +// served unfiltered. When the publish times can't be read, or the document +// can't be rewritten, the request fails instead, so a rate-limited or +// failing upstream can't let withheld versions through. + +// cooldownApplies reports whether cooldown withholds new versions of the +// package, taking package overrides and exemptions into account. +func (p *Proxy) cooldownApplies(ecosystem, name string) bool { + return p.cooldownEnabled() && p.Cooldown.For(ecosystem, canonicalPackagePURL(ecosystem, name)) > 0 +} + +// mustFilterMetadata reports whether the package's metadata may have +// versions to withhold, by cooldown or by the denylist, and so must not be +// served as upstream sent it. +func (p *Proxy) mustFilterMetadata(ecosystem, name string) bool { + return p.cooldownApplies(ecosystem, name) || + len(p.Denylist.Versions(canonicalPackagePURL(ecosystem, name))) != 0 +} + +// writeCooldownLookupError answers a metadata request whose publish times +// couldn't be read: 503 with Retry-After when the rate limit gate held the +// request, 502 otherwise. +func (p *Proxy) writeCooldownLookupError(w http.ResponseWriter, ecosystem, name string, err error) { + if writeUpstreamHeld(w, err) { + return + } + p.Logger.Warn("cooldown: could not read publish times", + "ecosystem", ecosystem, "package", name, "error", err) + http.Error(w, "failed to check cooldown", http.StatusBadGateway) +} diff --git a/internal/handler/cooldown_fail_closed_test.go b/internal/handler/cooldown_fail_closed_test.go new file mode 100644 index 00000000..ae3ae634 --- /dev/null +++ b/internal/handler/cooldown_fail_closed_test.go @@ -0,0 +1,271 @@ +package handler + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/git-pkgs/cooldown" + registryclient "github.com/git-pkgs/registries/client" + "github.com/git-pkgs/registries/fetch" +) + +// holdPaths returns a client that answers requests for the given paths with +// the rate limit gate's HeldError and sends the rest to upstream. +func holdPaths(paths ...string) *http.Client { + held := ®istryclient.HeldError{Host: "upstream.example", Until: time.Now().Add(30 * time.Second), RetryAfter: 30 * time.Second} + return &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + for _, p := range paths { + if r.URL.Path == p { + return nil, held + } + } + return http.DefaultTransport.RoundTrip(r) + })} +} + +func serveGet(t *testing.T, h http.Handler, path string) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w +} + +func wantStatus(t *testing.T, w *httptest.ResponseRecorder, status int, leaked string) { + t.Helper() + if w.Code != status { + t.Errorf("status = %d, want %d: %s", w.Code, status, w.Body.String()) + } + if status == http.StatusServiceUnavailable && w.Header().Get("Retry-After") == "" { + t.Error("503 without Retry-After") + } + if leaked != "" && strings.Contains(w.Body.String(), leaked) { + t.Errorf("response lists %q, which cooldown should have withheld", leaked) + } +} + +func TestGemCooldownFailsClosed(t *testing.T) { + index := "---\n1.0.0 |checksum:abc\n2.0.0 |checksum:def\n" + versionsStatus := http.StatusInternalServerError + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/api/v1/versions/") { + w.WriteHeader(versionsStatus) + return + } + _, _ = io.WriteString(w, index) + })) + defer upstream.Close() + + t.Run("versions API fails", func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "3d"} + h := NewGemHandlerWithUpstream(proxy, "http://proxy.test", upstream.URL) + wantStatus(t, serveGet(t, h.Routes(), "/info/testgem"), http.StatusBadGateway, "2.0.0") + }) + + t.Run("versions API held by rate limit", func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "3d"} + proxy.HTTPClient = holdPaths("/api/v1/versions/testgem.json") + h := NewGemHandlerWithUpstream(proxy, "http://proxy.test", upstream.URL) + wantStatus(t, serveGet(t, h.Routes(), "/info/testgem"), http.StatusServiceUnavailable, "2.0.0") + }) + + t.Run("exempt package needs no times", func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "3d", Packages: map[string]string{"pkg:gem/testgem": "0"}} + h := NewGemHandlerWithUpstream(proxy, "http://proxy.test", upstream.URL) + w := serveGet(t, h.Routes(), "/info/testgem") + if w.Code != http.StatusOK || w.Body.String() != index { + t.Errorf("status = %d, body = %q; want the index unchanged", w.Code, w.Body.String()) + } + }) +} + +func TestHexCooldownFailsClosed(t *testing.T) { + now := time.Now() + proto := buildHexSigned(buildHexPackage("testpkg", []string{testVersion100, "2.0.0"})) + apiJSON, _ := json.Marshal(hexPackageAPI{Releases: []hexRelease{ + {Version: testVersion100, InsertedAt: now.Add(-7 * 24 * time.Hour).Format(time.RFC3339Nano)}, + {Version: "2.0.0", InsertedAt: now.Add(-time.Hour).Format(time.RFC3339Nano)}, + }}) + + newUpstream := func(t *testing.T, packageBody []byte, apiStatus int) *httptest.Server { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/packages/testpkg": + _, _ = w.Write(packageBody) + case "/api/packages/testpkg": + w.WriteHeader(apiStatus) + _, _ = w.Write(apiJSON) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + return srv + } + handler := func(t *testing.T, srv *httptest.Server, client *http.Client) http.Handler { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "3d"} + if client != nil { + proxy.HTTPClient = client + } + return NewHexHandlerWithUpstreams(proxy, "http://proxy.test", srv.URL, srv.URL).Routes() + } + + t.Run("API fails", func(t *testing.T) { + srv := newUpstream(t, proto, http.StatusInternalServerError) + wantStatus(t, serveGet(t, handler(t, srv, nil), "/packages/testpkg"), http.StatusBadGateway, "") + }) + + t.Run("API held by rate limit", func(t *testing.T) { + srv := newUpstream(t, proto, http.StatusOK) + w := serveGet(t, handler(t, srv, holdPaths("/api/packages/testpkg")), "/packages/testpkg") + wantStatus(t, w, http.StatusServiceUnavailable, "") + }) + + t.Run("package can't be filtered", func(t *testing.T) { + srv := newUpstream(t, []byte("not gzip 2.0.0"), http.StatusOK) + wantStatus(t, serveGet(t, handler(t, srv, nil), "/packages/testpkg"), http.StatusBadGateway, "2.0.0") + }) + + t.Run("filtered", func(t *testing.T) { + srv := newUpstream(t, proto, http.StatusOK) + w := serveGet(t, handler(t, srv, nil), "/packages/testpkg") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + }) +} + +func TestPyPICooldownFailsClosed(t *testing.T) { + simple := `{"meta":{"api-version":"1.4"},"name":"ruff","files":[` + + `{"filename":"ruff-1.0.0.tar.gz","url":"https://files.pythonhosted.org/packages/ab/ruff-1.0.0.tar.gz"},` + + `{"filename":"ruff-2.0.0.tar.gz","url":"https://files.pythonhosted.org/packages/cd/ruff-2.0.0.tar.gz"}]}` + + tests := []struct { + name string + json func(*http.Request) (*http.Response, error) + status int + }{ + {"JSON API fails", func(r *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusInternalServerError, Body: http.NoBody, Request: r}, nil + }, http.StatusBadGateway}, + {"JSON API held by rate limit", func(*http.Request) (*http.Response, error) { + return nil, ®istryclient.HeldError{Host: "pypi.test", RetryAfter: 30 * time.Second} + }, http.StatusServiceUnavailable}, + {"JSON has no releases", func(r *http.Request) (*http.Response, error) { + return pypiHTTPResponse(r, "application/json", `{"info":{}}`), nil + }, http.StatusBadGateway}, + {"JSON API doesn't know the package", func(r *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusNotFound, Body: http.NoBody, Request: r}, nil + }, http.StatusOK}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h, proxy := setupPyPIHandler(t, func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/pypi/ruff/json" { + return tt.json(r) + } + return pypiHTTPResponse(r, pypiSimpleJSON, simple), nil + }) + proxy.Cooldown = &cooldown.Config{Default: "7d"} + + req := httptest.NewRequest(http.MethodGet, "/simple/ruff/", nil) + req.Header.Set("Accept", uvPyPIAccept) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + leaked := "ruff-2.0.0" + if tt.status == http.StatusOK { + leaked = "" + } + wantStatus(t, w, tt.status, leaked) + }) + } +} + +func TestPyPIDownloadCooldownFailsClosed(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer upstream.Close() + + proxy, _, _, fetcher := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "7d"} + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("sdist")), ContentType: "application/octet-stream"} + h := &PyPIHandler{proxy: proxy, upstreamURL: upstream.URL, proxyURL: "http://localhost"} + + w := serveGet(t, h.Routes(), "/packages/packages/ab/cd/ef0123456789/newpkg-2.0.0.tar.gz") + wantStatus(t, w, http.StatusBadGateway, "") + if fetcher.fetchCalled { + t.Error("fetched a file whose publish time couldn't be checked") + } +} + +func TestNPMDownloadCooldownFailsClosed(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer upstream.Close() + + proxy, _, _, fetcher := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "7d"} + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("tarball")), ContentType: "application/octet-stream"} + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + + w := serveGet(t, h.Routes(), "/leftpad/-/leftpad-2.0.0.tgz") + wantStatus(t, w, http.StatusBadGateway, "") + if fetcher.fetchCalled { + t.Error("fetched a tarball whose publish time couldn't be checked") + } +} + +// TestCooldownRewriteFailureFailsClosed checks that a metadata document the +// cooldown rewrite can't parse is refused rather than passed on, and still +// passed on when cooldown doesn't apply. +func TestCooldownRewriteFailureFailsClosed(t *testing.T) { + const broken = `{"name": "pkg", "versions": [ 2.0.0` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = io.WriteString(w, broken) + })) + defer upstream.Close() + + tests := []struct { + name string + purl string + handler func(*Proxy) http.Handler + path string + }{ + {"npm", "pkg:npm/pkg", func(p *Proxy) http.Handler { return NewNPMHandler(p, "http://proxy.test", upstream.URL).Routes() }, "/pkg"}, + {"composer", "pkg:composer/vendor/pkg", func(p *Proxy) http.Handler { + return NewComposerHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL).Routes() + }, "/p2/vendor/pkg.json"}, + {"pub", "pkg:pub/pkg", func(p *Proxy) http.Handler { + return NewPubHandlerWithUpstream(p, "http://proxy.test", upstream.URL).Routes() + }, "/api/packages/pkg"}, + {"pypi json", "pkg:pypi/pkg", func(p *Proxy) http.Handler { + return NewPyPIHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, "").Routes() + }, "/pypi/pkg/json"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "7d"} + wantStatus(t, serveGet(t, tt.handler(proxy), tt.path), http.StatusBadGateway, "2.0.0") + }) + t.Run(tt.name+" exempt", func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "7d", Packages: map[string]string{tt.purl: "0"}} + w := serveGet(t, tt.handler(proxy), tt.path) + if w.Code != http.StatusOK { + t.Errorf("status = %d, want 200 for a package cooldown doesn't apply to", w.Code) + } + }) + } +} diff --git a/internal/handler/gem.go b/internal/handler/gem.go index 13ea5281..b2ff6386 100644 --- a/internal/handler/gem.go +++ b/internal/handler/gem.go @@ -145,20 +145,19 @@ func gemVersionAndPlatform(s string) bool { // handleCompactIndex serves the compact index for a gem, filtering versions // based on cooldown when enabled. func (h *GemHandler) handleCompactIndex(w http.ResponseWriter, r *http.Request) { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - h.proxyCached(w, r) - return - } - name := r.PathValue("name") if name == "" { http.Error(w, "invalid gem name", http.StatusBadRequest) return } + if !h.proxy.cooldownApplies("gem", name) { + h.proxyCached(w, r) + return + } h.proxy.Logger.Info("gem compact index request with cooldown", "name", name) - indexResp, filteredVersions, err := h.fetchIndexAndVersions(r, name) + indexResp, filteredVersions, versionsErr, err := h.fetchIndexAndVersions(r, name) if err != nil { if writeUpstreamHeld(w, err) { return @@ -174,19 +173,18 @@ func (h *GemHandler) handleCompactIndex(w http.ResponseWriter, r *http.Request) return } - if filteredVersions == nil { - h.proxy.Logger.Warn("failed to fetch version timestamps, proxying unfiltered", "name", name) - h.proxy.relayResponse(w, r, indexResp, nil) + if versionsErr != nil { + h.proxy.writeCooldownLookupError(w, "gem", name, versionsErr) return } h.writeFilteredIndex(w, indexResp, name, filteredVersions) } -// fetchIndexAndVersions fetches the compact index and versions API concurrently. -// Returns the index response, a set of versions to filter (nil if versions API failed), -// and an error if the index fetch itself failed. -func (h *GemHandler) fetchIndexAndVersions(r *http.Request, name string) (*http.Response, map[string]bool, error) { +// fetchIndexAndVersions fetches the compact index and versions API +// concurrently. It returns the index response and the set of versions to +// filter, the versions API's error, and the index fetch's error. +func (h *GemHandler) fetchIndexAndVersions(r *http.Request, name string) (*http.Response, map[string]bool, error, error) { type versionsResult struct { filtered map[string]bool err error @@ -203,14 +201,9 @@ func (h *GemHandler) fetchIndexAndVersions(r *http.Request, name string) (*http. versionsRes := <-versionsCh if err != nil { - return nil, nil, err + return nil, nil, nil, err } - - if versionsRes.err != nil { - return indexResp, nil, nil - } - - return indexResp, versionsRes.filtered, nil + return indexResp, versionsRes.filtered, versionsRes.err, nil } // fetchCompactIndex fetches the compact index from upstream. diff --git a/internal/handler/gem_test.go b/internal/handler/gem_test.go index a615fad8..30470bc6 100644 --- a/internal/handler/gem_test.go +++ b/internal/handler/gem_test.go @@ -342,14 +342,12 @@ func TestGemCompactIndexVersionsAPIFails(t *testing.T) { w := httptest.NewRecorder() h.handleCompactIndex(w, req) - // Should still return OK with unfiltered content - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) + // Without publish times the index can't be filtered, so it isn't served. + if w.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want %d", w.Code, http.StatusBadGateway) } - - body := w.Body.String() - if !strings.Contains(body, "1.0.0") { - t.Error("expected unfiltered content when versions API fails") + if strings.Contains(w.Body.String(), "1.0.0") { + t.Error("served the unfiltered index when the versions API failed") } } diff --git a/internal/handler/hex.go b/internal/handler/hex.go index 847f955b..5b8e91ed 100644 --- a/internal/handler/hex.go +++ b/internal/handler/hex.go @@ -103,20 +103,15 @@ const hexAPIURL = "https://hex.pm" // when enabled. Since the protobuf format has no timestamps, we fetch them from the // Hex HTTP API concurrently. func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - h.proxyCached(w, r) - return - } - name := r.PathValue("name") - if name == "" { + if name == "" || !h.proxy.cooldownApplies("hex", name) { h.proxyCached(w, r) return } h.proxy.Logger.Info("hex package request with cooldown", "name", name) - protoResp, filteredVersions, err := h.fetchPackageAndVersions(r, name) + protoResp, filteredVersions, versionsErr, err := h.fetchPackageAndVersions(r, name) if err != nil { if writeUpstreamHeld(w, err) { return @@ -132,6 +127,11 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { return } + if versionsErr != nil { + h.proxy.writeCooldownLookupError(w, "hex", name, versionsErr) + return + } + body, err := io.ReadAll(protoResp.Body) if err != nil { http.Error(w, "failed to read response", http.StatusInternalServerError) @@ -139,7 +139,7 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { } if len(filteredVersions) == 0 { - // No versions to filter or couldn't get timestamps, pass through + // No versions in cooldown, pass through w.Header().Set(headerContentType, protoResp.Header.Get(headerContentType)) w.Header().Set("Content-Encoding", "gzip") _, _ = w.Write(body) @@ -148,10 +148,8 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { filtered, err := h.filterSignedPackage(body, filteredVersions) if err != nil { - h.proxy.Logger.Warn("failed to filter hex package, proxying original", "error", err) - w.Header().Set(headerContentType, protoResp.Header.Get(headerContentType)) - w.Header().Set("Content-Encoding", "gzip") - _, _ = w.Write(body) + h.proxy.Logger.Error("failed to filter hex package", "name", name, "error", err) + http.Error(w, "failed to filter package metadata", http.StatusBadGateway) return } @@ -160,8 +158,10 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(filtered) } -// fetchPackageAndVersions fetches the protobuf package and version timestamps concurrently. -func (h *HexHandler) fetchPackageAndVersions(r *http.Request, name string) (*http.Response, map[string]bool, error) { +// fetchPackageAndVersions fetches the protobuf package and version timestamps +// concurrently. It returns the package response and the set of versions to +// filter, the Hex API's error, and the package fetch's error. +func (h *HexHandler) fetchPackageAndVersions(r *http.Request, name string) (*http.Response, map[string]bool, error, error) { type versionsResult struct { filtered map[string]bool err error @@ -178,16 +178,9 @@ func (h *HexHandler) fetchPackageAndVersions(r *http.Request, name string) (*htt versionsRes := <-versionsCh if err != nil { - return nil, nil, err + return nil, nil, nil, err } - - if versionsRes.err != nil { - h.proxy.Logger.Warn("failed to fetch hex version timestamps, proxying unfiltered", - "name", name, "error", versionsRes.err) - return protoResp, nil, nil - } - - return protoResp, versionsRes.filtered, nil + return protoResp, versionsRes.filtered, versionsRes.err, nil } // fetchUpstreamPackage fetches the protobuf package from upstream. diff --git a/internal/handler/npm.go b/internal/handler/npm.go index ba1e2b1b..c2256d2f 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -277,7 +277,8 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) return } - if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { + if h.proxy.mustFilterMetadata("npm", packageName) { + h.proxy.Logger.Error("failed to filter npm metadata", "package", packageName, "error", err) JSONError(w, http.StatusBadGateway, "failed to filter package metadata") return } @@ -387,7 +388,8 @@ func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r * JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) return } - if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { + if h.proxy.mustFilterMetadata("npm", packageName) { + h.proxy.Logger.Error("failed to filter npm metadata", "package", packageName, "error", err) JSONError(w, http.StatusBadGateway, "failed to filter package metadata") return } @@ -923,7 +925,17 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { body, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), "npm", cacheKey, upstreamURL, "", h.packumentValidator(), h.packumentAccept()) return body, err }) - if h.versionInCooldown(packageName, version, metadata) { + inCooldown, err := h.versionInCooldown(packageName, version, metadata) + if err != nil { + if writeUpstreamHeld(w, err) { + return + } + h.proxy.Logger.Warn("cooldown: could not read npm publish time", + "package", packageName, "version", version, "error", err) + JSONError(w, http.StatusBadGateway, "failed to check cooldown") + return + } + if inCooldown { h.proxy.Logger.Info("cooldown: withholding npm tarball", "package", packageName, "version", version) JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) @@ -1031,7 +1043,8 @@ func validateTarballURL(raw, registryURL string) (string, error) { // proxy has never seen, persisting the parsed time so the packument is // fetched and parsed at most once per version. A version with no usable // publish time is allowed through, matching how applyCooldownFiltering -// treats it. +// treats it. A packument that can't be fetched or parsed is an error, so a +// failing upstream can't let a version through. // // The versions row is keyed by the plain version PURL, which does not say // which registry the version came from, so routed packages leave it alone: @@ -1040,39 +1053,38 @@ func validateTarballURL(raw, registryURL string) (string, error) { // for a private version of the same name, a private time would hold back the // public version, and moving a route to another upstream would keep the old // one's times. -func (h *NPMHandler) versionInCooldown(packageName, version string, metadata func() ([]byte, error)) bool { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - return false +func (h *NPMHandler) versionInCooldown(packageName, version string, metadata func() ([]byte, error)) (bool, error) { + if !h.proxy.cooldownApplies("npm", packageName) { + return false, nil } _, routed := h.routes.match(packageName) versionPURL := canonicalVersionPURL("npm", packageName, version) if !routed { if ver, err := h.proxy.DB.GetVersionByPURL(versionPURL); err == nil && ver != nil && ver.PublishedAt.Valid { - return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), ver.PublishedAt.Time) + return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), ver.PublishedAt.Time), nil } } body, err := metadata() + if errors.Is(err, ErrUpstreamNotFound) { + return false, nil + } if err != nil { - h.proxy.Logger.Warn("cooldown: could not fetch npm metadata for download check", - "package", packageName, "version", version, "error", err) - return false + return false, err } published, ok, err := lookupJSONString(body, "time", version) if err != nil { - h.proxy.Logger.Warn("cooldown: could not parse npm metadata for download check", - "package", packageName, "version", version, "error", err) - return false + return false, fmt.Errorf("parsing npm metadata: %w", err) } if !ok { - return false + return false, nil } publishedAt, err := time.Parse(time.RFC3339, published) if err != nil { - return false + return false, nil } if !routed { @@ -1082,7 +1094,7 @@ func (h *NPMHandler) versionInCooldown(packageName, version string, metadata fun } } - return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), publishedAt) + return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), publishedAt), nil } // npmMetadataPath returns the package name and the encoded registry path. diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index bbec4c65..08677767 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -561,10 +561,10 @@ func TestNPMDownloadCooldownDisabled(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) h := NewNPMHandler(proxy, "http://proxy.test", "") - if h.versionInCooldown("leftpad", testVersion100, func() ([]byte, error) { + if inCooldown, _ := h.versionInCooldown("leftpad", testVersion100, func() ([]byte, error) { t.Fatal("cooldown must not request metadata when disabled") return nil, nil - }) { + }); inCooldown { t.Error("versionInCooldown = true, want false when cooldown is not configured") } } diff --git a/internal/handler/pub.go b/internal/handler/pub.go index 1fc69977..05670f64 100644 --- a/internal/handler/pub.go +++ b/internal/handler/pub.go @@ -162,6 +162,11 @@ func (h *PubHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques rewritten, err := h.rewriteMetadata(name, body) if err != nil { + if h.proxy.mustFilterMetadata("pub", name) { + h.proxy.Logger.Error("failed to filter pub metadata", "package", name, "error", err) + http.Error(w, "failed to filter package metadata", http.StatusBadGateway) + return + } h.proxy.Logger.Warn("failed to rewrite metadata, proxying original", "error", err) w.Header().Set(headerContentType, "application/json") w.WriteHeader(http.StatusOK) diff --git a/internal/handler/pypi.go b/internal/handler/pypi.go index e2ca4dbf..b61bb3b1 100644 --- a/internal/handler/pypi.go +++ b/internal/handler/pypi.go @@ -121,10 +121,14 @@ func (h *PyPIHandler) handleSimplePackage(w http.ResponseWriter, r *http.Request return } - // When cooldown is enabled, fetch JSON metadata to get version timestamps + // When cooldown applies, fetch JSON metadata to get version timestamps var filteredVersions map[string]bool - if h.proxy.Cooldown != nil && h.proxy.Cooldown.Enabled() { - filteredVersions = h.fetchFilteredVersions(r, name) + if h.proxy.cooldownApplies("pypi", name) { + filteredVersions, err = h.fetchFilteredVersions(r, name) + if err != nil { + h.proxy.writeCooldownLookupError(w, "pypi", name, err) + return + } } for version := range h.proxy.Denylist.Versions(canonicalPackagePURL("pypi", name)) { if filteredVersions == nil { @@ -137,7 +141,8 @@ func (h *PyPIHandler) handleSimplePackage(w http.ResponseWriter, r *http.Request if isJSONMediaType(contentType) { rewritten, err = h.rewriteSimpleJSON(body, filteredVersions) if err != nil { - if len(h.proxy.Denylist.Versions(canonicalPackagePURL("pypi", name))) != 0 { + if len(filteredVersions) != 0 { + h.proxy.Logger.Error("failed to filter pypi simple json", "package", name, "error", err) http.Error(w, "failed to filter package metadata", http.StatusBadGateway) return } @@ -263,24 +268,29 @@ func ensureVaryAccept(header http.Header) { header.Add("Vary", "Accept") } -// fetchFilteredVersions fetches JSON metadata and returns a set of version strings -// that should be filtered out due to cooldown. -func (h *PyPIHandler) fetchFilteredVersions(r *http.Request, name string) map[string]bool { +// fetchFilteredVersions fetches JSON metadata and returns a set of version +// strings that should be filtered out due to cooldown. A package the JSON API +// doesn't know has no publish times to go by and gets an empty set; any other +// failure to read them is an error. +func (h *PyPIHandler) fetchFilteredVersions(r *http.Request, name string) (map[string]bool, error) { jsonURL := fmt.Sprintf("%s/pypi/%s/json", h.upstreamURL, name) body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "pypi", name+"/json", jsonURL) + if errors.Is(err, ErrUpstreamNotFound) { + return nil, nil + } if err != nil { - return nil + return nil, err } var metadata map[string]any if err := json.Unmarshal(body, &metadata); err != nil { - return nil + return nil, fmt.Errorf("parsing pypi json metadata: %w", err) } releases, ok := metadata["releases"].(map[string]any) if !ok { - return nil + return nil, errors.New("pypi json metadata has no releases") } packagePURL := canonicalPackagePURL("pypi", name) @@ -298,9 +308,9 @@ func (h *PyPIHandler) fetchFilteredVersions(r *http.Request, name string) map[st } if len(filtered) == 0 { - return nil + return nil, nil } - return filtered + return filtered, nil } // rewriteSimpleHTML rewrites package URLs in simple API HTML to point at this proxy. @@ -426,7 +436,8 @@ func (h *PyPIHandler) proxyAndRewriteJSON(w http.ResponseWriter, r *http.Request rewritten, err := h.rewriteJSONMetadata(body) if err != nil { - if len(h.proxy.Denylist.Versions(canonicalPackagePURL("pypi", r.PathValue("name")))) != 0 { + if h.proxy.mustFilterMetadata("pypi", r.PathValue("name")) { + h.proxy.Logger.Error("failed to filter pypi metadata", "url", upstreamURL, "error", err) http.Error(w, "failed to filter package metadata", http.StatusBadGateway) return } @@ -501,13 +512,14 @@ func (h *PyPIHandler) shouldFilterRelease(packagePURL string, files any) bool { // path without ever reading the index. // // A release whose upload time cannot be determined is allowed through, matching -// how fetchFilteredVersions treats it. -func (h *PyPIHandler) versionInCooldown(r *http.Request, name, version string) bool { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - return false +// how fetchFilteredVersions treats it. Failing to read the times is an error. +func (h *PyPIHandler) versionInCooldown(r *http.Request, name, version string) (bool, error) { + if !h.proxy.cooldownApplies("pypi", name) { + return false, nil } - return h.fetchFilteredVersions(r, name)[version] + filtered, err := h.fetchFilteredVersions(r, name) + return filtered[version], err } // rewriteFileEntries rewrites URLs in a list of file entries. @@ -612,11 +624,18 @@ func (h *PyPIHandler) handleDownload(w http.ResponseWriter, r *http.Request) { filename := parts[len(parts)-1] name, version := h.parseFilename(filename) - if name != "" && h.versionInCooldown(r, name, version) { - h.proxy.Logger.Info("cooldown: withholding pypi file", - "name", name, "version", version, "filename", filename) - http.Error(w, "not found", http.StatusNotFound) - return + if name != "" { + inCooldown, err := h.versionInCooldown(r, name, version) + if err != nil { + h.proxy.writeCooldownLookupError(w, "pypi", name, err) + return + } + if inCooldown { + h.proxy.Logger.Info("cooldown: withholding pypi file", + "name", name, "version", version, "filename", filename) + http.Error(w, "not found", http.StatusNotFound) + return + } } if name == "" { From aa0460a15b478942490aa70082fa1b04bf1d7693 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 24 Oct 2026 08:13:58 +0200 Subject: [PATCH 2/4] Bound Hex and gem cooldown reads by the metadata size limit With cooldown on, the Hex handler read the /packages response, its decompressed payload and the Hex API's JSON without a limit, and the gem handler decoded the versions API's JSON the same way. A misbehaving upstream could make the proxy hold any amount in memory, and a small gzip payload could expand without bound. These reads now go through ReadMetadata, so metadata_max_size applies to each, and to the Hex payload after decompression as the NuGet handler does. A response over the limit fails the request with 502. --- internal/handler/gem.go | 6 +++- internal/handler/gem_test.go | 29 ++++++++++++++++ internal/handler/hex.go | 16 +++++---- internal/handler/hex_test.go | 65 ++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/internal/handler/gem.go b/internal/handler/gem.go index b2ff6386..dbb6e08d 100644 --- a/internal/handler/gem.go +++ b/internal/handler/gem.go @@ -301,8 +301,12 @@ func (h *GemHandler) fetchFilteredVersions(r *http.Request, name string) (map[st return nil, fmt.Errorf("versions API returned %d", resp.StatusCode) } + body, err := h.proxy.ReadMetadata(resp.Body) + if err != nil { + return nil, err + } var versions []gemVersion - if err := json.NewDecoder(resp.Body).Decode(&versions); err != nil { + if err := json.Unmarshal(body, &versions); err != nil { return nil, err } diff --git a/internal/handler/gem_test.go b/internal/handler/gem_test.go index 30470bc6..a13d819a 100644 --- a/internal/handler/gem_test.go +++ b/internal/handler/gem_test.go @@ -397,3 +397,32 @@ func TestGemFetchFilteredVersions(t *testing.T) { _ = fmt.Sprintf // silence unused import } + +func TestGemCooldownVersionsSizeLimit(t *testing.T) { + versionsJSON, _ := json.Marshal([]gemVersion{ + {Number: "2.0.0", Platform: "ruby", CreatedAt: time.Now().Add(-time.Hour).Format(time.RFC3339)}, + }) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/api/v1/versions/") { + _, _ = w.Write(versionsJSON) + _, _ = w.Write([]byte(strings.Repeat(" ", 4096))) + return + } + _, _ = w.Write([]byte("---\n2.0.0 |checksum:abc\n")) + })) + defer upstream.Close() + + proxy := testProxy() + proxy.Cooldown = &cooldown.Config{Default: "3d"} + proxy.MetadataMaxSize = 1024 + h := &GemHandler{proxy: proxy, upstreamURL: upstream.URL} + + req := httptest.NewRequest(http.MethodGet, "/info/testgem", nil) + req.SetPathValue("name", "testgem") + w := httptest.NewRecorder() + h.handleCompactIndex(w, req) + + if w.Code != http.StatusBadGateway { + t.Errorf("status = %d, want %d", w.Code, http.StatusBadGateway) + } +} diff --git a/internal/handler/hex.go b/internal/handler/hex.go index 5b8e91ed..2d86686c 100644 --- a/internal/handler/hex.go +++ b/internal/handler/hex.go @@ -5,7 +5,6 @@ import ( "compress/gzip" "encoding/json" "fmt" - "io" "net/http" "strings" "time" @@ -132,9 +131,10 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { return } - body, err := io.ReadAll(protoResp.Body) + body, err := h.proxy.ReadMetadata(protoResp.Body) if err != nil { - http.Error(w, "failed to read response", http.StatusInternalServerError) + h.proxy.Logger.Error("failed to read hex package", "name", name, "error", err) + http.Error(w, "failed to read response", http.StatusBadGateway) return } @@ -224,8 +224,12 @@ func (h *HexHandler) fetchFilteredVersions(r *http.Request, name string) (map[st return nil, fmt.Errorf("hex API returned %d", resp.StatusCode) } + body, err := h.proxy.ReadMetadata(resp.Body) + if err != nil { + return nil, err + } var pkg hexPackageAPI - if err := json.NewDecoder(resp.Body).Decode(&pkg); err != nil { + if err := json.Unmarshal(body, &pkg); err != nil { return nil, err } @@ -253,12 +257,12 @@ func (h *HexHandler) fetchFilteredVersions(r *http.Request, name string) (map[st // filters releases from the Package payload, and re-encodes as gzipped protobuf // (without the original signature since the payload has changed). func (h *HexHandler) filterSignedPackage(gzippedData []byte, filteredVersions map[string]bool) ([]byte, error) { - // Decompress gzip + // Decompress gzip, with the metadata limit applied to the decompressed size gr, err := gzip.NewReader(bytes.NewReader(gzippedData)) if err != nil { return nil, err } - signed, err := io.ReadAll(gr) + signed, err := h.proxy.ReadMetadata(gr) if err != nil { return nil, err } diff --git a/internal/handler/hex_test.go b/internal/handler/hex_test.go index 92120bd6..d448b760 100644 --- a/internal/handler/hex_test.go +++ b/internal/handler/hex_test.go @@ -3,6 +3,7 @@ package handler import ( "bytes" "compress/gzip" + cryptorand "crypto/rand" "encoding/json" "io" "log/slog" @@ -329,3 +330,67 @@ func TestHexHandlePackagesWithoutCooldown(t *testing.T) { t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) } } + +func TestHexCooldownMetadataSizeLimit(t *testing.T) { + const limit = 1024 + now := time.Now() + releases := []hexRelease{ + {Version: testVersion100, InsertedAt: now.Add(-7 * 24 * time.Hour).Format(time.RFC3339Nano)}, + {Version: "2.0.0", InsertedAt: now.Add(-time.Hour).Format(time.RFC3339Nano)}, + } + apiJSON, _ := json.Marshal(hexPackageAPI{Releases: releases}) + pkg := buildHexPackage("testpkg", []string{testVersion100, "2.0.0"}) + proto := buildHexSigned(pkg) + + // An unknown bytes field pads the payload: it compresses to a few bytes + // but decompresses past the limit. + padded := protowire.AppendTag(append([]byte(nil), pkg...), 4, protowire.BytesType) + padded = protowire.AppendBytes(padded, make([]byte, 64*limit)) + bomb := buildHexSigned(padded) + if len(bomb) >= limit { + t.Fatalf("compressed package is %d bytes, want under %d", len(bomb), limit) + } + // Padding that doesn't compress makes the response itself too large. + noise := make([]byte, 2*limit) + _, _ = cryptorand.Read(noise) + large := protowire.AppendTag(append([]byte(nil), pkg...), 4, protowire.BytesType) + large = buildHexSigned(protowire.AppendBytes(large, noise)) + if len(large) <= limit { + t.Fatalf("compressed package is %d bytes, want over %d", len(large), limit) + } + + tests := []struct { + name string + pkg, api []byte + }{ + {"package over limit", large, apiJSON}, + {"package decompresses past limit", bomb, apiJSON}, + {"API response over limit", proto, append(append([]byte(nil), apiJSON...), bytes.Repeat([]byte(" "), 2*limit)...)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/packages/testpkg": + _, _ = w.Write(tt.pkg) + case "/api/packages/testpkg": + _, _ = w.Write(tt.api) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "3d"} + proxy.MetadataMaxSize = limit + h := NewHexHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL) + + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/packages/testpkg", nil)) + if w.Code != http.StatusBadGateway { + t.Errorf("status = %d, want %d", w.Code, http.StatusBadGateway) + } + }) + } +} From 224bce8f51cccdffde12a34604cd5fdde399ce3d Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 25 Oct 2026 12:31:33 +0100 Subject: [PATCH 3/4] Serve cached downloads when cooldown can't read publish times PyPI and npm downloads look up the publish time before the cache. When that lookup fails, a file that is already cached is now served, and only a cache miss fails with 502 or 503. The proxy keeps serving what it has while the upstream is unreachable, and nothing new is fetched unchecked. Also drops the unattached comment block in cooldown_fail_closed.go. --- internal/handler/cooldown_fail_closed.go | 23 +++++++--- internal/handler/cooldown_fail_closed_test.go | 42 +++++++++++++++++++ internal/handler/npm.go | 3 ++ internal/handler/pypi.go | 4 +- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/internal/handler/cooldown_fail_closed.go b/internal/handler/cooldown_fail_closed.go index 3a0a7f44..da600c1e 100644 --- a/internal/handler/cooldown_fail_closed.go +++ b/internal/handler/cooldown_fail_closed.go @@ -2,11 +2,6 @@ package handler import "net/http" -// Metadata for a package that cooldown or the denylist applies to is never -// served unfiltered. When the publish times can't be read, or the document -// can't be rewritten, the request fails instead, so a rate-limited or -// failing upstream can't let withheld versions through. - // cooldownApplies reports whether cooldown withholds new versions of the // package, taking package overrides and exemptions into account. func (p *Proxy) cooldownApplies(ecosystem, name string) bool { @@ -32,3 +27,21 @@ func (p *Proxy) writeCooldownLookupError(w http.ResponseWriter, ecosystem, name "ecosystem", ecosystem, "package", name, "error", err) http.Error(w, "failed to check cooldown", http.StatusBadGateway) } + +// serveCachedDespiteCooldownError serves a download from the cache when its +// publish time couldn't be read, and reports whether it did. A cached file was +// already let through once, so the proxy keeps serving it while the upstream +// is unreachable; only a cache miss fails. +func (p *Proxy) serveCachedDespiteCooldownError(w http.ResponseWriter, r *http.Request, ecosystem, name, version, filename string, lookupErr error) bool { + if p.versionDenied(ecosystem, name, version) { + return false + } + cached, err := p.GetCachedArtifact(r.Context(), ecosystem, name, version, filename) + if err != nil || cached == nil { + return false + } + p.Logger.Warn("cooldown: could not read publish times, serving cached file", + "ecosystem", ecosystem, "package", name, "version", version, "error", lookupErr) + ServeArtifactRequest(w, r, cached) + return true +} diff --git a/internal/handler/cooldown_fail_closed_test.go b/internal/handler/cooldown_fail_closed_test.go index ae3ae634..dc7cb98a 100644 --- a/internal/handler/cooldown_fail_closed_test.go +++ b/internal/handler/cooldown_fail_closed_test.go @@ -225,6 +225,48 @@ func TestNPMDownloadCooldownFailsClosed(t *testing.T) { } } +// TestDownloadCooldownErrorServesCachedFile checks that a file already in the +// cache is still served when its publish time can't be read, while an uncached +// one fails. +func TestDownloadCooldownErrorServesCachedFile(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer upstream.Close() + + tests := []struct { + ecosystem, name, version, filename, path string + handler func(*Proxy) http.Handler + }{ + {"pypi", "newpkg", "2.0.0", "newpkg-2.0.0.tar.gz", "/packages/packages/ab/cd/ef0123456789/newpkg-2.0.0.tar.gz", + func(p *Proxy) http.Handler { + return (&PyPIHandler{proxy: p, upstreamURL: upstream.URL, downloadURL: upstream.URL, proxyURL: "http://localhost"}).Routes() + }}, + {"npm", "leftpad", "2.0.0", "leftpad-2.0.0.tgz", "/leftpad/-/leftpad-2.0.0.tgz", + func(p *Proxy) http.Handler { return NewNPMHandler(p, "http://proxy.test", upstream.URL).Routes() }}, + } + for _, tt := range tests { + t.Run(tt.ecosystem, func(t *testing.T) { + proxy, db, store, fetcher := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "7d"} + h := tt.handler(proxy) + + seedPackage(t, db, store, tt.ecosystem, tt.name, tt.version, tt.filename, "cached file") + w := serveGet(t, h, tt.path) + wantStatus(t, w, http.StatusOK, "") + if w.Body.String() != "cached file" { + t.Errorf("body = %q, want the cached file", w.Body.String()) + } + + other := strings.ReplaceAll(tt.path, "2.0.0", "3.0.0") + wantStatus(t, serveGet(t, h, other), http.StatusBadGateway, "") + if fetcher.fetchCalled { + t.Error("fetched a file whose publish time couldn't be checked") + } + }) + } +} + // TestCooldownRewriteFailureFailsClosed checks that a metadata document the // cooldown rewrite can't parse is refused rather than passed on, and still // passed on when cooldown doesn't apply. diff --git a/internal/handler/npm.go b/internal/handler/npm.go index c2256d2f..fbeb7cf9 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -927,6 +927,9 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { }) inCooldown, err := h.versionInCooldown(packageName, version, metadata) if err != nil { + if h.proxy.serveCachedDespiteCooldownError(w, r, "npm", packageName, version, h.routes.cacheFilename(packageName, filename), err) { + return + } if writeUpstreamHeld(w, err) { return } diff --git a/internal/handler/pypi.go b/internal/handler/pypi.go index b61bb3b1..e22471c4 100644 --- a/internal/handler/pypi.go +++ b/internal/handler/pypi.go @@ -627,7 +627,9 @@ func (h *PyPIHandler) handleDownload(w http.ResponseWriter, r *http.Request) { if name != "" { inCooldown, err := h.versionInCooldown(r, name, version) if err != nil { - h.proxy.writeCooldownLookupError(w, "pypi", name, err) + if !h.proxy.serveCachedDespiteCooldownError(w, r, "pypi", name, version, filename, err) { + h.proxy.writeCooldownLookupError(w, "pypi", name, err) + } return } if inCooldown { From 4402075916696722f75f12b5f7c00bc91d0d9175 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 25 Oct 2026 12:32:17 +0100 Subject: [PATCH 4/4] Document serving cached downloads when publish times can't be read --- docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index 4712d696..d0e44553 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1042,7 +1042,7 @@ Maven caveats: - Maven Central lists versions in version order, not in the order they were published. A patch to an older release line published after a newer release that is out of cooldown stays listed while it is in cooldown, but its files are still refused. - SNAPSHOT versions are not held back. -RubyGems, Hex and PyPI keep their publish times outside the metadata clients read: the RubyGems versions API, the Hex HTTP API (`upstream.hex_api`) and the PyPI JSON API. The proxy fetches them alongside each compact index, package or simple page it filters, and for PyPI downloads. Cooldown fails closed: when it applies to a package and those times can't be fetched or parsed, or the metadata can't be rewritten, the request fails with 502, or with 503 and `Retry-After` while the upstream is rate limiting the proxy, instead of serving the metadata unfiltered. The same goes for npm, Composer and pub.dev metadata that can't be rewritten, and for npm tarball downloads whose publish time can't be read. Packages exempted with a duration of `0` are served as before and don't need the times. A PyPI package the JSON API returns 404 for has no times to go by and is served unfiltered. +RubyGems, Hex and PyPI keep their publish times outside the metadata clients read: the RubyGems versions API, the Hex HTTP API (`upstream.hex_api`) and the PyPI JSON API. The proxy fetches them alongside each compact index, package or simple page it filters, and for PyPI downloads. Cooldown fails closed: when it applies to a package and those times can't be fetched or parsed, or the metadata can't be rewritten, the request fails with 502, or with 503 and `Retry-After` while the upstream is rate limiting the proxy, instead of serving the metadata unfiltered. The same goes for npm, Composer and pub.dev metadata that can't be rewritten, and for npm tarball downloads whose publish time can't be read. A PyPI file or npm tarball that is already cached is still served when its publish time can't be read; only a download that would have to be fetched fails. Packages exempted with a duration of `0` are served as before and don't need the times. A PyPI package the JSON API returns 404 for has no times to go by and is served unfiltered. 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.