npm: read the stored publish time in the cooldown download check

internal/database/database_test.go

@@ -1099,3 +1099,56 @@ func BenchmarkMigrateSchemaFullyMigrated(b *testing.B) {
 		}
 	}
 }
+
+func TestVersionPublishedAtPreserved(t *testing.T) {
+	runWithBothDatabases(t, func(t *testing.T, db *DB) {
+		publishedAt := time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC)
+
+		if err := db.SetVersionPublishedAt("pkg:npm/leftpad@1.0.0", "pkg:npm/leftpad", publishedAt); err != nil {
+			t.Fatalf("SetVersionPublishedAt failed: %v", err)
+		}
+
+		got, err := db.GetVersionByPURL("pkg:npm/leftpad@1.0.0")
+		if err != nil || got == nil {
+			t.Fatalf("GetVersionByPURL failed: %v", err)
+		}
+		if !got.PublishedAt.Valid || !got.PublishedAt.Time.Equal(publishedAt) {
+			t.Fatalf("PublishedAt = %v, want %v", got.PublishedAt, publishedAt)
+		}
+
+		// An upsert that carries no publish time (the artifact cache path)
+		// must not erase the stored value.
+		if err := db.UpsertVersion(&Version{
+			PURL:        "pkg:npm/leftpad@1.0.0",
+			PackagePURL: "pkg:npm/leftpad",
+		}); err != nil {
+			t.Fatalf("UpsertVersion failed: %v", err)
+		}
+
+		got, err = db.GetVersionByPURL("pkg:npm/leftpad@1.0.0")
+		if err != nil || got == nil {
+			t.Fatalf("GetVersionByPURL after upsert failed: %v", err)
+		}
+		if !got.PublishedAt.Valid || !got.PublishedAt.Time.Equal(publishedAt) {
+			t.Fatalf("PublishedAt after null upsert = %v, want %v preserved", got.PublishedAt, publishedAt)
+		}
+
+		// An upsert that does carry a publish time still updates it.
+		later := publishedAt.Add(24 * time.Hour)
+		if err := db.UpsertVersion(&Version{
+			PURL:        "pkg:npm/leftpad@1.0.0",
+			PackagePURL: "pkg:npm/leftpad",
+			PublishedAt: sql.NullTime{Time: later, Valid: true},
+		}); err != nil {
+			t.Fatalf("UpsertVersion with publish time failed: %v", err)
+		}
+
+		got, err = db.GetVersionByPURL("pkg:npm/leftpad@1.0.0")
+		if err != nil || got == nil {
+			t.Fatalf("GetVersionByPURL after second upsert failed: %v", err)
+		}
+		if !got.PublishedAt.Valid || !got.PublishedAt.Time.Equal(later) {
+			t.Fatalf("PublishedAt after valued upsert = %v, want %v", got.PublishedAt, later)
+		}
+	})
+}

internal/database/queries.go

@@ -141,7 +141,7 @@ func (db *DB) UpsertVersion(v *Version) error {
 			ON CONFLICT(purl) DO UPDATE SET
 				license = EXCLUDED.license,
 				integrity = EXCLUDED.integrity,
-				published_at = EXCLUDED.published_at,
+				published_at = COALESCE(EXCLUDED.published_at, versions.published_at),
 				yanked = EXCLUDED.yanked,
 				enriched_at = EXCLUDED.enriched_at,
 				updated_at = EXCLUDED.updated_at
@@ -154,7 +154,7 @@ func (db *DB) UpsertVersion(v *Version) error {
 			ON CONFLICT(purl) DO UPDATE SET
 				license = excluded.license,
 				integrity = excluded.integrity,
-				published_at = excluded.published_at,
+				published_at = COALESCE(excluded.published_at, published_at),
 				yanked = excluded.yanked,
 				enriched_at = excluded.enriched_at,
 				updated_at = excluded.updated_at
@@ -171,6 +171,38 @@ func (db *DB) UpsertVersion(v *Version) error {
 	return nil
 }
 
+// SetVersionPublishedAt records a version's publish time, creating the
+// versions row if the proxy has not seen the version yet. It only writes
+// published_at, so it never disturbs enrichment data on an existing row.
+func (db *DB) SetVersionPublishedAt(versionPURL, packagePURL string, publishedAt time.Time) error {
+	now := time.Now()
+	var query string
+
+	if db.dialect == DialectPostgres {
+		query = `
+			INSERT INTO versions (purl, package_purl, published_at, created_at, updated_at)
+			VALUES ($1, $2, $3, $4, $5)
+			ON CONFLICT(purl) DO UPDATE SET
+				published_at = EXCLUDED.published_at,
+				updated_at = EXCLUDED.updated_at
+		`
+	} else {
+		query = `
+			INSERT INTO versions (purl, package_purl, published_at, created_at, updated_at)
+			VALUES (?, ?, ?, ?, ?)
+			ON CONFLICT(purl) DO UPDATE SET
+				published_at = excluded.published_at,
+				updated_at = excluded.updated_at
+		`
+	}
+
+	_, err := db.Exec(query, versionPURL, packagePURL, publishedAt, now, now)
+	if err != nil {
+		return fmt.Errorf("setting version publish time: %w", err)
+	}
+	return nil
+}
+
 // Artifact queries
 
 func (db *DB) GetArtifact(versionPURL, filename string) (*Artifact, error) {

internal/handler/handler.go

@@ -71,6 +71,12 @@ func canonicalPackagePURL(ecosystem, name string) string {
 	return purl.MakePURLString(ecosystem, name, "")
 }
 
+// canonicalVersionPURL returns a versioned PURL in canonical form, matching
+// the keys the artifact cache writes to the versions table.
+func canonicalVersionPURL(ecosystem, name, version string) string {
+	return purl.MakePURLString(ecosystem, name, version)
+}
+
 const contentTypeJSON = "application/json"
 
 const headerAcceptEncoding = "Accept-Encoding"

internal/handler/npm.go

@@ -302,14 +302,22 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) {
 // 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.
+// A version's publish time is immutable, so the check reads the stored
+// versions row first and only falls back to the packument for a version the
+// 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.
 func (h *NPMHandler) versionInCooldown(r *http.Request, packageName, version string) bool {
 	if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() {
 		return false
 	}
 
+	versionPURL := canonicalVersionPURL("npm", packageName, version)
+	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)
+	}
+
 	upstreamURL := fmt.Sprintf("%s/%s", h.upstreamURL, url.PathEscape(packageName))
 
 	body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, upstreamURL, contentTypeJSON)
@@ -338,6 +346,11 @@ func (h *NPMHandler) versionInCooldown(r *http.Request, packageName, version str
 		return false
 	}
 
+	if err := h.proxy.DB.SetVersionPublishedAt(versionPURL, canonicalPackagePURL("npm", packageName), publishedAt); err != nil {
+		h.proxy.Logger.Warn("cooldown: could not store npm publish time",
+			"package", packageName, "version", version, "error", err)
+	}
+
 	return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), publishedAt)
 }
 

internal/handler/npm_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -532,3 +533,102 @@ func TestNPMDownloadCooldownDisabled(t *testing.T) {
 		t.Error("versionInCooldown = true, want false when cooldown is not configured")
 	}
 }
+
+func TestNPMDownloadCooldownUsesStoredPublishTime(t *testing.T) {
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		t.Error("metadata must not be fetched when the publish time is already stored")
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer upstream.Close()
+
+	tests := []struct {
+		name        string
+		version     string
+		publishedAt time.Time
+		wantStatus  int
+	}{
+		{"stored time before the window serves the tarball", testVersion100, time.Now().Add(-30 * 24 * time.Hour), http.StatusOK},
+		{"stored time inside the window is withheld", "2.0.0", time.Now().Add(-1 * time.Hour), http.StatusNotFound},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			proxy, db, _, 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",
+			}
+
+			if err := db.SetVersionPublishedAt("pkg:npm/leftpad@"+tt.version, "pkg:npm/leftpad", tt.publishedAt); err != nil {
+				t.Fatalf("seeding publish time failed: %v", err)
+			}
+
+			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)
+			}
+		})
+	}
+}
+
+func TestNPMDownloadCooldownFetchesMetadataOnce(t *testing.T) {
+	now := time.Now()
+	packument := `{
+		"name": "leftpad",
+		"dist-tags": {"latest": "1.0.0"},
+		"time": {
+			"1.0.0": "` + now.Add(-30*24*time.Hour).Format(time.RFC3339) + `"
+		},
+		"versions": {"1.0.0": {}}
+	}`
+
+	var metadataRequests atomic.Int64
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		metadataRequests.Add(1)
+		w.Header().Set("Content-Type", contentTypeJSON)
+		_, _ = io.WriteString(w, packument)
+	}))
+	defer upstream.Close()
+
+	proxy, _, _, fetcher := setupTestProxy(t)
+	proxy.HTTPClient = upstream.Client()
+	proxy.Cooldown = &cooldown.Config{Default: "7d"}
+
+	h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL)
+	srv := httptest.NewServer(h.Routes())
+	defer srv.Close()
+
+	// The first download parses the packument once and persists the publish
+	// time; caching the artifact afterwards upserts the versions row without a
+	// publish time, which must not erase the stored value. The second download
+	// must answer from the stored time alone.
+	for i := 0; i < 2; i++ {
+		fetcher.artifact = &fetch.Artifact{
+			Body:        io.NopCloser(strings.NewReader("tarball data")),
+			ContentType: "application/octet-stream",
+		}
+		resp, err := http.Get(srv.URL + "/leftpad/-/leftpad-" + testVersion100 + ".tgz")
+		if err != nil {
+			t.Fatalf("request %d failed: %v", i+1, err)
+		}
+		_ = resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			t.Fatalf("request %d status = %d, want %d", i+1, resp.StatusCode, http.StatusOK)
+		}
+	}
+
+	if got := metadataRequests.Load(); got != 1 {
+		t.Errorf("metadata requests = %d, want 1", got)
+	}
+}