diff --git a/docs/configuration.md b/docs/configuration.md index 49156494..d0e44553 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. 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. 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..da600c1e --- /dev/null +++ b/internal/handler/cooldown_fail_closed.go @@ -0,0 +1,47 @@ +package handler + +import "net/http" + +// 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) +} + +// 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 new file mode 100644 index 00000000..dc7cb98a --- /dev/null +++ b/internal/handler/cooldown_fail_closed_test.go @@ -0,0 +1,313 @@ +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") + } +} + +// 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. +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..dbb6e08d 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. @@ -308,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 a615fad8..a13d819a 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") } } @@ -399,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 847f955b..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" @@ -103,20 +102,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,14 +126,20 @@ func (h *HexHandler) handlePackages(w http.ResponseWriter, r *http.Request) { return } - body, err := io.ReadAll(protoResp.Body) + if versionsErr != nil { + h.proxy.writeCooldownLookupError(w, "hex", name, versionsErr) + return + } + + 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 } 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 - } - - 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 nil, nil, nil, err } - - return protoResp, versionsRes.filtered, nil + return protoResp, versionsRes.filtered, versionsRes.err, nil } // fetchUpstreamPackage fetches the protobuf package from upstream. @@ -231,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 } @@ -260,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) + } + }) + } +} diff --git a/internal/handler/npm.go b/internal/handler/npm.go index ba1e2b1b..fbeb7cf9 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,20 @@ 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 h.proxy.serveCachedDespiteCooldownError(w, r, "npm", packageName, version, h.routes.cacheFilename(packageName, filename), err) { + return + } + 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 +1046,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 +1056,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 +1097,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..e22471c4 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,20 @@ 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 { + if !h.proxy.serveCachedDespiteCooldownError(w, r, "pypi", name, version, filename, err) { + 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 == "" {