From 74447f9bab751d53cc64305060f16517cddd4338 Mon Sep 17 00:00:00 2001 From: Oscar Broman Date: Sat, 8 Aug 2026 20:05:45 +0400 Subject: [PATCH] Enforce cooldown on artifact downloads Cooldown filtering only ran when rewriting metadata, so a version could be missing from the npm packument and the PyPI simple index while its tarball stayed reachable. Lockfiles record artifact URLs verbatim, so npm ci and pinned pip requirements reach handleDownload without ever requesting metadata. The shared artifact path has no publish time to check against, since updateCacheDB upserts versions without PublishedAt and the column is only set by enrichment. Each handler now resolves the publish time from metadata it already fetches and returns 404 while a version is inside the window. Versions with no usable publish time are still served, as they are when filtering metadata. --- internal/handler/npm.go | 51 +++++++++++++++++++++++ internal/handler/npm_test.go | 78 +++++++++++++++++++++++++++++++++++ internal/handler/pypi.go | 22 ++++++++++ internal/handler/pypi_test.go | 56 +++++++++++++++++++++++++ 4 files changed, 207 insertions(+) diff --git a/internal/handler/npm.go b/internal/handler/npm.go index ee9b59c..81367e8 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -265,6 +265,13 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { h.proxy.Logger.Info("npm download request", "package", packageName, "version", version, "filename", filename) + if h.versionInCooldown(r, packageName, version) { + h.proxy.Logger.Info("cooldown: withholding npm tarball", + "package", packageName, "version", version) + JSONError(w, http.StatusNotFound, "version not found") + return + } + downloadURL := fmt.Sprintf( "%s/%s/-/%s", h.upstreamURL, @@ -287,6 +294,50 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { ServeArtifact(w, result) } +// versionInCooldown reports whether a version is still inside the cooldown +// window. Filtering the packument is not enough on its own: tarball URLs are +// predictable and lockfiles record them directly, so `npm ci` reaches the +// download path without ever requesting metadata. +// +// The packument is served from the metadata cache, so this normally costs no +// extra upstream request. A version with no usable publish time is allowed +// through, matching how applyCooldownFiltering treats it. +func (h *NPMHandler) versionInCooldown(r *http.Request, packageName, version string) bool { + if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { + return false + } + + upstreamURL := fmt.Sprintf("%s/%s", h.upstreamURL, url.PathEscape(packageName)) + + body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, upstreamURL, contentTypeJSON) + if err != nil { + h.proxy.Logger.Warn("cooldown: could not fetch npm metadata for download check", + "package", packageName, "version", version, "error", err) + return false + } + + var metadata struct { + Time map[string]string `json:"time"` + } + if err := json.Unmarshal(body, &metadata); err != nil { + h.proxy.Logger.Warn("cooldown: could not parse npm metadata for download check", + "package", packageName, "version", version, "error", err) + return false + } + + published, ok := metadata.Time[version] + if !ok { + return false + } + + publishedAt, err := time.Parse(time.RFC3339, published) + if err != nil { + return false + } + + return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), publishedAt) +} + func escapeNPMDownloadPackage(packageName string) string { scope, name, scoped := strings.Cut(packageName, "/") if scoped && strings.HasPrefix(scope, "@") && len(scope) > 1 && name != "" && !strings.Contains(name, "/") { diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index e0257dd..3e2ebe0 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -454,3 +454,81 @@ func TestNPMHandlerMetadataNotFound(t *testing.T) { t.Errorf("status = %d, want %d", w.Code, http.StatusNotFound) } } + +func TestNPMDownloadCooldown(t *testing.T) { + now := time.Now() + packument := `{ + "name": "leftpad", + "dist-tags": {"latest": "2.0.0"}, + "time": { + "1.0.0": "` + now.Add(-30*24*time.Hour).Format(time.RFC3339) + `", + "2.0.0": "` + now.Add(-1*time.Hour).Format(time.RFC3339) + `" + }, + "versions": {"1.0.0": {}, "2.0.0": {}} + }` + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", contentTypeJSON) + _, _ = io.WriteString(w, packument) + })) + defer upstream.Close() + + tests := []struct { + name string + version string + wantStatus int + }{ + {"published before the window serves the tarball", testVersion100, http.StatusOK}, + {"published inside the window is withheld", "2.0.0", http.StatusNotFound}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy, _, _, fetcher := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.Cooldown = &cooldown.Config{Default: "7d"} + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("tarball data")), + ContentType: "application/octet-stream", + } + + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + srv := httptest.NewServer(h.Routes()) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/leftpad/-/leftpad-" + tt.version + ".tgz") + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != tt.wantStatus { + t.Errorf("status = %d, want %d", resp.StatusCode, tt.wantStatus) + } + if tt.wantStatus == http.StatusNotFound && fetcher.fetchCalled { + t.Error("fetched a version that is still inside the cooldown window") + } + }) + } +} + +func TestNPMDownloadCooldownDisabled(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("metadata must not be fetched when cooldown is disabled") + w.WriteHeader(http.StatusInternalServerError) + })) + defer upstream.Close() + + proxy, _, _, fetcher := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("tarball data")), + ContentType: "application/octet-stream", + } + + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + + if h.versionInCooldown(httptest.NewRequest(http.MethodGet, "/", nil), "leftpad", testVersion100) { + t.Error("versionInCooldown = true, want false when cooldown is not configured") + } +} diff --git a/internal/handler/pypi.go b/internal/handler/pypi.go index 713f6c8..7875cdd 100644 --- a/internal/handler/pypi.go +++ b/internal/handler/pypi.go @@ -311,6 +311,21 @@ func (h *PyPIHandler) shouldFilterRelease(packagePURL string, files any) bool { return !publishedAt.IsZero() && !h.proxy.Cooldown.IsAllowed("pypi", packagePURL, publishedAt) } +// versionInCooldown reports whether a version is still inside the cooldown +// window. Filtering the simple index is not enough on its own: file URLs are +// recorded in lockfiles and requirements pins, so pip can reach the download +// 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 + } + + return h.fetchFilteredVersions(r, name)[version] +} + // rewriteFileEntries rewrites URLs in a list of file entries. func (h *PyPIHandler) rewriteFileEntries(files any) { filesArr, ok := files.([]any) @@ -417,6 +432,13 @@ 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 == "" { // Can't determine name/version, use hash as identifier name = fmt.Sprintf("_hash_%s", hashPath(path)) diff --git a/internal/handler/pypi_test.go b/internal/handler/pypi_test.go index 5ae76ca..6bbcf3e 100644 --- a/internal/handler/pypi_test.go +++ b/internal/handler/pypi_test.go @@ -236,3 +236,59 @@ func TestPyPIHandler_DownloadCacheMiss(t *testing.T) { t.Error("expected fetcher to be called on cache miss") } } + +func TestPyPIDownloadCooldown(t *testing.T) { + now := time.Now() + releases := `{"releases": { + "1.0.0": [{"upload_time_iso_8601": "` + now.Add(-30*24*time.Hour).Format(time.RFC3339) + `"}], + "2.0.0": [{"upload_time_iso_8601": "` + now.Add(-1*time.Hour).Format(time.RFC3339) + `"}] + }}` + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", contentTypeJSON) + _, _ = io.WriteString(w, releases) + })) + defer upstream.Close() + + tests := []struct { + name string + filename string + wantStatus int + }{ + {"published before the window serves the file", "newpkg-1.0.0.tar.gz", http.StatusOK}, + {"published inside the window is withheld", "newpkg-2.0.0.tar.gz", http.StatusNotFound}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy, _, _, fetcher := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.Cooldown = &cooldown.Config{Default: "7d"} + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("sdist data")), + ContentType: "application/octet-stream", + } + + h := &PyPIHandler{ + proxy: proxy, + upstreamURL: upstream.URL, + proxyURL: "http://localhost", + } + srv := httptest.NewServer(h.Routes()) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/packages/packages/ab/cd/ef0123456789/" + tt.filename) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != tt.wantStatus { + t.Errorf("status = %d, want %d", resp.StatusCode, tt.wantStatus) + } + if tt.wantStatus == http.StatusNotFound && fetcher.fetchCalled { + t.Error("fetched a version that is still inside the cooldown window") + } + }) + } +}