diff --git a/docs/configuration.md b/docs/configuration.md index 4da6f634..660421f0 100644 --- a/docs/configuration.md +++ b/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. diff --git a/internal/handler/composer.go b/internal/handler/composer.go index 643b287f..768b89ae 100644 --- a/internal/handler/composer.go +++ b/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 diff --git a/internal/handler/composer_download_lookup_test.go b/internal/handler/composer_download_lookup_test.go index 5ac4c75c..56350004 100644 --- a/internal/handler/composer_download_lookup_test.go +++ b/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) + } +}