Serve cached Composer releases without a metadata lookup

docs/configuration.md

@@ -349,7 +349,7 @@ upstream:
 - An npm route URL is the registry base that serves `/{package}`. Tarball URLs in its packuments must be on the same host and below the same path.
 - A routed package is not looked up anywhere else. If the private registry does not have it, the client gets a 404. If the registry fails or rejects the credentials, the client gets a 502. A package with the same name on the public registry is never served in its place (dependency confusion).
 - Metadata and artifacts of routed packages are cached under keys that include the route's URL. Entries cached from the public registry before a route was added are never served for a routed package.
-- With cooldown enabled, the publish time of a routed npm version always comes from the route's metadata. Times recorded for the public package of the same name, or for a route's previous URL, are not used, and routed downloads do not record times of their own.
+- With cooldown enabled, the publish time of a routed npm or Composer version always comes from the route's metadata. Times recorded for the public package of the same name, or for a route's previous URL, are not used, and routed downloads do not record times of their own.
 - Composer metadata of routed packages carries an empty `notification-url`, so Composer does not report their installs to the default upstream's `notify-batch` endpoint.
 - Use `upstream.auth` for the registry's credentials. The proxy has no authentication of its own, so every client that can reach it can download routed packages.
 - Search (`/composer/search.json`) and the package list (`/composer/packages/list.json`) still come from the default upstream only.
@@ -1209,7 +1209,7 @@ cache_metadata: true
 
 Or via environment variable: `PROXY_CACHE_METADATA=true`.
 
-Composer archive downloads look up their upstream URL in the same cached metadata, so a cached archive is served without asking Packagist again and stays installable while it is down. Only a version or file the cached metadata does not list yet is looked up upstream.
+Composer archive downloads look up their upstream URL in the same cached metadata, so an archive is resolved without asking Packagist again and stays installable while it is down. Only a version or file the cached metadata does not list yet is looked up upstream. With or without `cache_metadata`, a cached archive of a tagged release from the default repository is served without any metadata lookup; with cooldown enabled it is checked against the publish time recorded when the archive was first resolved. Branch (dev) versions, routed packages and archives picked by a query string such as `?sha=` are always resolved from the metadata.
 
 The `proxy mirror` command always enables metadata caching regardless of this setting.
 

internal/handler/composer.go

@@ -383,13 +383,8 @@ func (h *ComposerHandler) shouldFilterVersion(packagePURL, packageName, version
 		return false
 	}
 
-	var timeStr string
-	if err := json.Unmarshal(published, &timeStr); err != nil {
-		return false
-	}
-
-	publishedAt, err := time.Parse(time.RFC3339, timeStr)
-	if err != nil {
+	publishedAt, ok := composerPublishTime(published)
+	if !ok {
 		return false
 	}
 
@@ -402,6 +397,17 @@ func (h *ComposerHandler) shouldFilterVersion(packagePURL, packageName, version
 	return false
 }
 
+// composerPublishTime parses the raw "time" field of a version, reporting
+// false when it is missing or not an RFC 3339 string.
+func composerPublishTime(published []byte) (time.Time, bool) {
+	var timeStr string
+	if err := json.Unmarshal(published, &timeStr); err != nil {
+		return time.Time{}, false
+	}
+	publishedAt, err := time.Parse(time.RFC3339, timeStr)
+	return publishedAt, err == nil
+}
+
 // disableDefaultNotification stops Composer from reporting installs of a routed
 // package to the default repository's notify-batch URL, which would disclose
 // the package name to it. Composer only falls back to notify-batch when a
@@ -633,6 +639,8 @@ func (h *ComposerHandler) handleDownload(w http.ResponseWriter, r *http.Request)
 	requested := path.Base(r.URL.EscapedPath())
 	if r.URL.RawQuery != "" {
 		requested += "?" + r.URL.RawQuery
+	} else if h.serveCachedRelease(w, r, packageName, version, filename) {
+		return
 	}
 	files := h.metadataFilesForVersion(vendor, pkg, version)
 	dist, downloadURL, ok := h.cachedDist(r.Context(), files, packageName, version, requested)
@@ -641,6 +649,7 @@ func (h *ComposerHandler) handleDownload(w http.ResponseWriter, r *http.Request)
 			return
 		}
 	}
+	h.recordPublishTime(packageName, version, dist.published)
 
 	// Lock files name archive URLs directly, so a version filtered out of the
 	// metadata still has to be withheld here.
@@ -682,6 +691,77 @@ func (h *ComposerHandler) handleDownload(w http.ResponseWriter, r *http.Request)
 	ServeArtifactRequest(w, r, result)
 }
 
+// releaseTimeStored reports whether the publish time of version is kept in
+// the versions table, so a download can be checked against cooldown without
+// the metadata. That holds for tagged releases of packages from the default
+// repository. A branch is left out: its archive and time change with every
+// push. So are routed packages, as for npm: the versions row is keyed by the
+// plain PURL, which does not say which repository the version came from, so
+// a time read from the default repository would be trusted for a private
+// version of the same name, and the other way round.
+func (h *ComposerHandler) releaseTimeStored(packageName, version string) bool {
+	if h.proxy.DB == nil || isDevVersion(version) {
+		return false
+	}
+	_, routed := h.routes.match(packageName)
+	return !routed
+}
+
+// serveCachedRelease serves a tagged release's archive from the cache
+// without looking up its dist in the metadata, and reports whether it wrote a
+// response. The archive was only cached under its name after that name was
+// checked against the metadata, and the cache verifies it against the digest
+// recorded then. Cooldown is checked against the publish time recorded when
+// the dist was first resolved; without one, as for an archive cached by an
+// older proxy, it returns false and the caller takes the metadata path, which
+// records it. Requests with a query string, which pick a commit, always take
+// that path.
+func (h *ComposerHandler) serveCachedRelease(w http.ResponseWriter, r *http.Request, packageName, version, filename string) bool {
+	if !h.releaseTimeStored(packageName, version) {
+		return false
+	}
+	if h.proxy.Cooldown != nil && h.proxy.Cooldown.Enabled() {
+		ver, err := h.proxy.DB.GetVersionByPURL(canonicalVersionPURL("composer", packageName, version))
+		if err != nil || ver == nil || !ver.PublishedAt.Valid {
+			return false
+		}
+		if !h.proxy.Cooldown.IsAllowed("composer", canonicalPackagePURL("composer", packageName), ver.PublishedAt.Time) {
+			h.proxy.Logger.Info("cooldown: withholding composer archive",
+				"package", packageName, "version", version)
+			http.Error(w, "version not found", http.StatusNotFound)
+			return true
+		}
+	}
+	cached, err := h.proxy.GetCachedArtifact(r.Context(), "composer", packageName, version, filename)
+	if err != nil {
+		h.proxy.serveArtifactError(w, err, "failed to read cached package")
+		return true
+	}
+	if cached == nil {
+		return false
+	}
+	ServeArtifactRequest(w, r, cached)
+	return true
+}
+
+// recordPublishTime stores the publish time a release's metadata lists, for
+// serveCachedRelease. It is stored whether or not cooldown is enabled, so
+// enabling it later does not send cached archives back to the metadata.
+func (h *ComposerHandler) recordPublishTime(packageName, version string, published []byte) {
+	if !h.releaseTimeStored(packageName, version) {
+		return
+	}
+	publishedAt, ok := composerPublishTime(published)
+	if !ok {
+		return
+	}
+	if err := h.proxy.DB.SetVersionPublishedAt(canonicalVersionPURL("composer", packageName, version),
+		canonicalPackagePURL("composer", packageName), publishedAt); err != nil {
+		h.proxy.Logger.Warn("failed to store composer publish time",
+			"package", packageName, "version", version, "error", err)
+	}
+}
+
 // discardOtherCommit clears a cached branch archive named for commit that was
 // fetched from another commit's URL. Proxy versions before the pinned-commit
 // fix cached the branch head under whatever commit a lock file asked for. The

internal/handler/composer_download_lookup_test.go

@@ -10,23 +10,29 @@ import (
 	"testing"
 	"time"
 
+	"github.com/git-pkgs/cooldown"
 	"github.com/git-pkgs/registries/fetch"
 )
 
 // composerLookupUpstream serves v/p's metadata listing versions and the
 // archives they point at, counts metadata and archive requests, and fails
-// every request while down is set.
+// every request while down is set. A version listed in published carries that
+// publish time.
 type composerLookupUpstream struct {
-	h        http.Handler
-	versions atomic.Value // []string
-	down     atomic.Bool
-	metadata atomic.Int32
-	archives atomic.Int32
+	h         http.Handler
+	handler   *ComposerHandler
+	proxy     *Proxy
+	versions  atomic.Value // []string
+	published map[string]time.Time
+	down      atomic.Bool
+	metadata  atomic.Int32
+	archives  atomic.Int32
+	routeURL  string
 }
 
 func setupComposerLookup(t *testing.T, cacheMetadata bool, versions ...string) *composerLookupUpstream {
 	t.Helper()
-	u := &composerLookupUpstream{}
+	u := &composerLookupUpstream{published: map[string]time.Time{}}
 	u.versions.Store(versions)
 	var srv *httptest.Server
 	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -39,7 +45,11 @@ func setupComposerLookup(t *testing.T, cacheMetadata bool, versions ...string) *
 			u.metadata.Add(1)
 			var entries []string
 			for _, v := range u.versions.Load().([]string) {
-				entries = append(entries, fmt.Sprintf(`{"version":%q,"dist":{"url":%q,"type":"zip"}}`, v, srv.URL+"/archives/"+v+".zip"))
+				var published string
+				if at, ok := u.published[v]; ok {
+					published = fmt.Sprintf(`"time":%q,`, at.Format(time.RFC3339))
+				}
+				entries = append(entries, fmt.Sprintf(`{"version":%q,%s"dist":{"url":%q,"type":"zip"}}`, v, published, srv.URL+"/archives/"+v+".zip"))
 			}
 			_, _ = fmt.Fprintf(w, `{"packages":{"v/p":[%s]}}`, strings.Join(entries, ","))
 		case r.URL.Path == "/p2/v/p~dev.json":
@@ -60,10 +70,19 @@ func setupComposerLookup(t *testing.T, cacheMetadata bool, versions ...string) *
 	fetcher := fetch.NewFetcher(fetch.WithHTTPClient(srv.Client()), fetch.WithMaxRetries(0))
 	proxy.Fetcher = fetcher
 	t.Cleanup(func() { _ = fetcher.Close() })
-	u.h = NewComposerHandlerWithUpstreams(proxy, "http://proxy", srv.URL, srv.URL).Routes()
+	u.proxy = proxy
+	u.handler = NewComposerHandlerWithUpstreams(proxy, "http://proxy", srv.URL, srv.URL)
+	u.h = u.handler.Routes()
+	u.routeURL = srv.URL
 	return u
 }
 
+// route sends v/p to a custom repository, which is the same server under
+// another name.
+func (u *composerLookupUpstream) route() {
+	u.h = u.handler.WithPackageRoutes(map[string]string{"v/*": u.routeURL}).Routes()
+}
+
 func (u *composerLookupUpstream) get(t *testing.T, target string) *httptest.ResponseRecorder {
 	t.Helper()
 	w := httptest.NewRecorder()
@@ -126,3 +145,169 @@ func TestComposerDownloadWithoutMetadataCache(t *testing.T) {
 		t.Errorf("upstream metadata requests = %d, want 1", n)
 	}
 }
+
+// storedPublishTime returns the publish time recorded for v/p at version.
+func (u *composerLookupUpstream) storedPublishTime(t *testing.T, version string) (time.Time, bool) {
+	t.Helper()
+	ver, err := u.proxy.DB.GetVersionByPURL(canonicalVersionPURL("composer", "v/p", version))
+	if err != nil {
+		t.Fatalf("reading versions row: %v", err)
+	}
+	if ver == nil || !ver.PublishedAt.Valid {
+		return time.Time{}, false
+	}
+	return ver.PublishedAt.Time, true
+}
+
+func (u *composerLookupUpstream) wantMetadataRequests(t *testing.T, want int32) {
+	t.Helper()
+	if n := u.metadata.Load(); n != want {
+		t.Errorf("upstream metadata requests = %d, want %d", n, want)
+	}
+}
+
+// TestComposerCachedReleaseSkipsMetadata checks that a tagged release whose
+// archive is cached is served without asking upstream for the metadata
+// again, with and without cache_metadata, and with cooldown, which takes the
+// publish time recorded when the archive was first resolved.
+func TestComposerCachedReleaseSkipsMetadata(t *testing.T) {
+	for _, tt := range []struct {
+		name          string
+		cacheMetadata bool
+		cooldown      bool
+	}{
+		{"metadata not cached", false, false},
+		{"metadata cached", true, false},
+		{"cooldown", false, true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			u := setupComposerLookup(t, tt.cacheMetadata, "1.0.0")
+			u.published["1.0.0"] = time.Now().Add(-30 * 24 * time.Hour)
+			if tt.cooldown {
+				u.proxy.Cooldown = &cooldown.Config{Default: "3d"}
+			}
+			u.download(t, "1.0.0")
+			// With cache_metadata the cached copy expires; the archive must
+			// not need it.
+			u.proxy.MetadataTTL = 0
+			u.download(t, "1.0.0")
+			u.download(t, "1.0.0")
+			u.wantMetadataRequests(t, 1)
+			if n := u.archives.Load(); n != 1 {
+				t.Errorf("upstream archive requests = %d, want 1", n)
+			}
+		})
+	}
+}
+
+// TestComposerDownloadStoresPublishTime checks that resolving a tagged
+// release records its publish time, so later downloads can check cooldown
+// without the metadata.
+func TestComposerDownloadStoresPublishTime(t *testing.T) {
+	u := setupComposerLookup(t, false, "1.0.0")
+	published := time.Date(2024, 5, 6, 7, 8, 9, 0, time.UTC)
+	u.published["1.0.0"] = published
+	u.download(t, "1.0.0")
+	got, ok := u.storedPublishTime(t, "1.0.0")
+	if !ok || !got.Equal(published) {
+		t.Errorf("stored publish time = %v, %v, want %v", got, ok, published)
+	}
+}
+
+// TestComposerCachedReleaseCooldownFromStoredTime checks that a cached
+// archive whose stored publish time is still inside the cooldown window is
+// withheld without the metadata, including when cooldown was enabled or
+// raised after the archive was cached.
+func TestComposerCachedReleaseCooldownFromStoredTime(t *testing.T) {
+	u := setupComposerLookup(t, false, "1.0.0", "2.0.0")
+	u.published["1.0.0"] = time.Now().Add(-5 * 24 * time.Hour)
+	u.published["2.0.0"] = time.Now().Add(-1 * time.Hour)
+	u.download(t, "2.0.0")
+	u.proxy.Cooldown = &cooldown.Config{Default: "3d"}
+	u.download(t, "1.0.0")
+
+	if w := u.get(t, "/files/v/p/2.0.0/2.0.0.zip"); w.Code != http.StatusNotFound {
+		t.Errorf("2.0.0 after enabling cooldown: status %d, want 404", w.Code)
+	}
+	u.proxy.Cooldown = &cooldown.Config{Default: "7d"}
+	if w := u.get(t, "/files/v/p/1.0.0/1.0.0.zip"); w.Code != http.StatusNotFound {
+		t.Errorf("1.0.0 after raising cooldown: status %d, want 404", w.Code)
+	}
+	u.wantMetadataRequests(t, 2)
+}
+
+// TestComposerCachedReleaseWithoutStoredTime checks that with cooldown a
+// cached archive the proxy recorded no publish time for, such as one cached
+// by an older version, is checked against the metadata, which then records
+// the time.
+func TestComposerCachedReleaseWithoutStoredTime(t *testing.T) {
+	u := setupComposerLookup(t, false, "1.0.0")
+	u.published["1.0.0"] = time.Now().Add(-1 * time.Hour)
+	u.download(t, "1.0.0")
+	if _, err := u.proxy.DB.Exec(`UPDATE versions SET published_at = NULL`); err != nil {
+		t.Fatal(err)
+	}
+
+	u.proxy.Cooldown = &cooldown.Config{Default: "3d"}
+	if w := u.get(t, "/files/v/p/1.0.0/1.0.0.zip"); w.Code != http.StatusNotFound {
+		t.Errorf("status %d, want 404", w.Code)
+	}
+	u.wantMetadataRequests(t, 2)
+	if _, ok := u.storedPublishTime(t, "1.0.0"); !ok {
+		t.Error("publish time not recorded from the metadata")
+	}
+}
+
+// TestComposerCachedDevVersionUsesMetadata checks that a branch, whose
+// archive and publish time change with every push, is still resolved from
+// the metadata on each download and records no publish time.
+func TestComposerCachedDevVersionUsesMetadata(t *testing.T) {
+	u := setupComposerLookup(t, false, "dev-main")
+	u.published["dev-main"] = time.Now().Add(-30 * 24 * time.Hour)
+	u.download(t, "dev-main")
+	before := u.metadata.Load()
+	u.download(t, "dev-main")
+	if u.metadata.Load() == before {
+		t.Error("cached branch archive served without consulting the metadata")
+	}
+	if _, ok := u.storedPublishTime(t, "dev-main"); ok {
+		t.Error("publish time recorded for a branch")
+	}
+}
+
+// TestComposerCachedRoutedReleaseUsesMetadata checks that a routed package
+// keeps resolving every download from its route's metadata: the versions
+// table is keyed by a PURL that does not say which repository the version
+// came from, so it neither stores nor trusts a publish time for it.
+func TestComposerCachedRoutedReleaseUsesMetadata(t *testing.T) {
+	u := setupComposerLookup(t, false, "1.0.0")
+	u.published["1.0.0"] = time.Now().Add(-1 * time.Hour)
+	u.route()
+	u.download(t, "1.0.0")
+	u.download(t, "1.0.0")
+	u.wantMetadataRequests(t, 2)
+	if _, ok := u.storedPublishTime(t, "1.0.0"); ok {
+		t.Error("publish time recorded for a routed package")
+	}
+
+	// A time stored for the public package of the same name is not used.
+	if err := u.proxy.DB.SetVersionPublishedAt(canonicalVersionPURL("composer", "v/p", "1.0.0"),
+		canonicalPackagePURL("composer", "v/p"), time.Now().Add(-30*24*time.Hour)); err != nil {
+		t.Fatal(err)
+	}
+	u.proxy.Cooldown = &cooldown.Config{Default: "3d"}
+	if w := u.get(t, "/files/v/p/1.0.0/1.0.0.zip"); w.Code != http.StatusNotFound {
+		t.Errorf("routed release in cooldown: status %d, want 404", w.Code)
+	}
+}
+
+// TestComposerCachedReleaseDenied checks that the denylist still blocks a
+// cached release served without the metadata.
+func TestComposerCachedReleaseDenied(t *testing.T) {
+	u := setupComposerLookup(t, false, "1.0.0")
+	u.download(t, "1.0.0")
+	setTestDenylist(t, u.proxy, "pkg:composer/v/p@1.0.0")
+	if w := u.get(t, "/files/v/p/1.0.0/1.0.0.zip"); w.Code != http.StatusForbidden {
+		t.Errorf("status %d, want 403", w.Code)
+	}
+}