From d073fdcd52fcf0e988f78941650e37578d8791a4 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 08:00:14 +0100 Subject: [PATCH 1/6] Close npm, PyPI and pub cooldown gaps in metadata and downloads npm: a tarball request for a version the cached packument did not list yet (cached within metadata_ttl before the release) found no publish time and was let through cooldown, and fetched without integrity. When cooldown applies and the cached packument lacks the version, fetch it again and decide on what upstream lists now; if it can't be fetched, fail as when a publish time can't be read. npm: cooldown only moved dist-tags.latest. Other tags kept naming versions in cooldown, while the denylist removes such tags. Cooldown now removes them too, and drops latest when no version is left, in both the in-memory and streamed rewrites. PyPI: the simple JSON rewrite removed filtered files but left their versions in the PEP 700 versions list. Remove them there too. pub: latest was never rewritten, so its archive_url pointed at pub.dev, it kept naming a version in cooldown when no version was left, and the replacement was the last list entry, which may be a prerelease. Point latest's archive_url at the proxy, replace a held back latest with the newest stable version left (or the newest prerelease), and drop it when nothing is left. --- docs/configuration.md | 6 ++- internal/handler/npm.go | 71 ++++++++++++++++++++------ internal/handler/npm_attestations.go | 2 +- internal/handler/npm_tarball_test.go | 57 +++++++++++++++++++++ internal/handler/pub.go | 50 ++++++++++++------ internal/handler/pub_test.go | 55 ++++++++++++++++++++ internal/handler/pypi.go | 13 +++++ internal/handler/pypi_test.go | 26 ++++++++++ internal/metadata/npm/metadata.go | 22 ++++++-- internal/metadata/npm/metadata_test.go | 36 +++++++++++++ 10 files changed, 296 insertions(+), 42 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 584896bc..93de4100 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1240,7 +1240,9 @@ RubyGems, Hex and PyPI keep their publish times outside the metadata clients rea For RubyGems and Hex, both documents go through the metadata cache. With [`cache_metadata`](#metadata-caching) they are served from it within `metadata_ttl` without asking the upstream, revalidated with their ETags after it, and filtered from the last cached copies, marked stale, while the upstream is down; without it they are still fetched once for concurrent requests. Each document has its own TTL, so the compact index or Hex package can be newer than the cached times and list a version they don't have. The times are then fetched again before filtering, and if that fails, such versions are left out until the times can be read. The filtered index or package carries an ETag of its own, so Bundler and Mix revalidate it with `304 Not Modified`. Publish times are compared with the current time on each request, and the filtered result is kept until the first version it held back leaves cooldown (see [rewritten metadata cache](#rewritten-metadata-cache)); a newly published version shows up within `metadata_ttl`, as with other cached metadata. Upstream errors are answered as for other cached metadata (404, or 502) rather than relayed. -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. When `latest` is in cooldown, the packument tags the newest version out of cooldown instead, and `npm dist-tag ls` shows the same tags. +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. When `latest` is in cooldown, the packument tags the newest version out of cooldown instead; other tags that name a version in cooldown are removed, as for the denylist, and `npm dist-tag ls` shows the same tags. A tarball request for a version the cached packument doesn't list yet fetches the packument again, within `metadata_ttl` or not, and is decided on what the upstream lists now; if the packument can't be fetched, the download is not let through, as when a publish time can't be read. + +In pub.dev package metadata, when `latest` is in cooldown it names the newest stable version out of cooldown instead, or the newest prerelease when no stable version is left, and it is left out when every version is in cooldown. Conda cooldown covers `repodata.json`, `current_repodata.json`, `repodata_from_packages.json` and sharded repodata (CEP 16). Other forms of the repodata, such as `repodata.json.zst`, `repodata.json.bz2` and `repodata.jlap` patches, would list versions still in cooldown, so with cooldown on they are answered 404 and conda and mamba fall back to `repodata.json`. With `cache_metadata`, `repodata.json` is read through the metadata cache as it is without cooldown: within `metadata_ttl` the stored copy is filtered as it is read from storage, after it the upstream is asked with its ETag, and while the upstream is down the stale copy is filtered. Once a document has been filtered, the proxy remembers the ETag of the result until the first entry it left out leaves cooldown, so a client revalidating with `If-None-Match` gets `304 Not Modified` without the document being filtered or sent again. The first response for each document has no ETag, as it is sent while it is filtered. Without `cache_metadata`, `repodata.json` is fetched and filtered on every request. 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`. @@ -1296,7 +1298,7 @@ Names and versions are compared the way each ecosystem treats them: channel only. Denied versions are removed from npm packuments, Cargo sparse indexes, and PyPI -simple HTML/JSON and release metadata, including responses served from cached +simple HTML/JSON (files and the JSON `versions` list) and release metadata, including responses served from cached metadata during an upstream outage. No publication timestamp or enabled cooldown is required. npm tags pointing at denied versions are removed; `latest` is moved to the highest remaining stable version when available. Other tags are not diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 50757a37..0e9f32ac 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -344,7 +344,7 @@ func (h *NPMHandler) serveStreamedPackument(w http.ResponseWriter, r *http.Reque fullRequired := h.packumentValidator() != nil options := h.npmRewriteOptions(packageName, nil) var rewritten rewrittenMetadata - err, fetchErr := h.readStoredPackument(r.Context(), upstreamURL, accept, metadataCacheKey, func(entry *database.MetadataCacheEntry) (err error) { + err, fetchErr := h.readStoredPackument(r.Context(), upstreamURL, accept, metadataCacheKey, false, func(entry *database.MetadataCacheEntry) (err error) { rewritten, err = h.proxy.streamedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, entry, func(open func() (io.ReadCloser, error), out io.Writer) error { var scan *npm.Scan err := readStored(open, func(in io.Reader) (err error) { @@ -413,7 +413,18 @@ var errInMemory = errors.New("npm packument needs the in-memory path") // fetching it into storage first when needed. It returns errInMemory when // metadata isn't cached in storage, and ErrUpstreamNotFound and upstream // errors as they come. -func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, cacheKey string) (*database.MetadataCacheEntry, error) { +// With refresh, a stored copy within its TTL is fetched again rather than +// served. +func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool) (*database.MetadataCacheEntry, error) { + if refresh && h.proxy.DB != nil { + if row, _ := h.proxy.DB.GetMetadataCache("npm", cacheKey); usableStoredMetadata(row) { + entry, err := h.proxy.restoreStoredMetadata(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator(), row) + if errors.Is(err, errMetadataNotStored) { + return nil, errInMemory + } + return entry, err + } + } entry, err := h.proxy.storedMetadataDocument(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator()) if errors.Is(err, errMetadataNotStored) { return nil, errInMemory @@ -426,8 +437,8 @@ func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, // or no longer has the digest its row records, the packument is fetched // again and read once more. fetchErr is what storedPackument returned when the packument // couldn't be fetched, and err is what read returned. -func (h *NPMHandler) readStoredPackument(ctx context.Context, packumentURL, accept, cacheKey string, read func(*database.MetadataCacheEntry) error) (err, fetchErr error) { - entry, fetchErr := h.storedPackument(ctx, packumentURL, accept, cacheKey) +func (h *NPMHandler) readStoredPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool, read func(*database.MetadataCacheEntry) error) (err, fetchErr error) { + entry, fetchErr := h.storedPackument(ctx, packumentURL, accept, cacheKey, refresh) if fetchErr != nil { return nil, fetchErr } @@ -475,7 +486,7 @@ func (h *NPMHandler) streamedVersionMetadata(ctx context.Context, packageName, s fullRequired := h.packumentValidator() != nil options := h.npmRewriteOptions(packageName, nil) var m rewrittenMetadata - err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, func(entry *database.MetadataCacheEntry) (err error) { + err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, false, func(entry *database.MetadataCacheEntry) (err error) { m, err = h.proxy.streamedRewrite(ctx, "npm", h.proxyURL, metadataCacheKey, entry, func(open func() (io.ReadCloser, error), out io.Writer) error { var scan *npm.Scan err := readStored(open, func(in io.Reader) (err error) { @@ -776,7 +787,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { } registryURL, cacheKey := h.sourceFor(packageName) metadata := sync.OnceValues(func() (npmVersionData, error) { - return h.versionData(r.Context(), registryURL+"/"+url.PathEscape(packageName), cacheKey, version) + return h.versionData(r.Context(), packageName, registryURL+"/"+url.PathEscape(packageName), cacheKey, version) }) inCooldown, err := h.versionInCooldown(packageName, version, metadata) if err != nil { @@ -883,10 +894,27 @@ type npmVersionData struct { // versionData reads what a tarball download needs about version from the // packument: from storage in one pass when metadata is cached there, and // otherwise from the packument read into memory. -func (h *NPMHandler) versionData(ctx context.Context, packumentURL, cacheKey, version string) (npmVersionData, error) { +// +// A packument cached within its TTL can predate the version. Under cooldown a +// missing entry would let the version through with no publish time, so the +// packument is fetched again and the version decided on what upstream lists +// now, as the Composer handler does. +func (h *NPMHandler) versionData(ctx context.Context, packageName, packumentURL, cacheKey, version string) (npmVersionData, error) { + data, fromCache, err := h.readVersionData(ctx, packumentURL, cacheKey, version, false) + if err == nil && data.entry == nil && fromCache && h.proxy.cooldownApplies("npm", packageName) { + h.proxy.Logger.Info("cooldown: cached npm metadata lacks version, fetching it again", + "package", packageName, "version", version) + data, _, err = h.readVersionData(ctx, packumentURL, cacheKey, version, true) + } + return data, err +} + +// readVersionData implements versionData. fromCache reports whether the +// packument may have been a cached copy; refresh fetches it from upstream. +func (h *NPMHandler) readVersionData(ctx context.Context, packumentURL, cacheKey, version string, refresh bool) (data npmVersionData, fromCache bool, err error) { accept := h.packumentAccept() var found npm.VersionEntry - err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, func(entry *database.MetadataCacheEntry) error { + err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, refresh, func(entry *database.MetadataCacheEntry) error { return readStored(func() (io.ReadCloser, error) { return h.proxy.openStoredMetadata(ctx, entry) }, func(in io.Reader) (err error) { found, err = npm.LookupVersion(in, version) return err @@ -894,23 +922,32 @@ func (h *NPMHandler) versionData(ctx context.Context, packumentURL, cacheKey, ve }) switch { case fetchErr == nil && err == nil && (!found.Abbreviated || h.packumentValidator() == nil): - return npmVersionData{entry: found.Entry, published: found.Published}, nil + return npmVersionData{entry: found.Entry, published: found.Published}, !refresh, nil case fetchErr == nil && err != nil: h.proxy.Logger.Warn("failed to read stored npm metadata, reading it in memory", "url", packumentURL, "error", err) case fetchErr != nil && !errors.Is(fetchErr, errInMemory): - return npmVersionData{}, fetchErr + return npmVersionData{}, false, fetchErr } - body, _, _, err := h.proxy.fetchOrCacheMetadata(ctx, "npm", cacheKey, packumentURL, "", h.packumentValidator(), accept) - if err != nil { - return npmVersionData{}, err + var res metadataResult + if refresh { + res = h.proxy.refreshMetadata(ctx, "npm", cacheKey, packumentURL, accept, "", h.packumentValidator()) + } else { + res = h.proxy.fetchMetadataDocument(ctx, "npm", cacheKey, packumentURL, accept, h.packumentValidator()) } - entry, _ := jsonscan.Lookup(body, "versions", version) - published, _, err := jsonscan.LookupString(body, "time", version) + if res.err != nil { + return npmVersionData{}, false, res.err + } + // The cached copy is what lacked the version, so it can't decide it. + if refresh && res.stale { + return npmVersionData{}, false, errors.New("npm metadata could not be fetched again") + } + entry, _ := jsonscan.Lookup(res.body, "versions", version) + published, _, err := jsonscan.LookupString(res.body, "time", version) if err != nil { - return npmVersionData{}, fmt.Errorf("parsing npm metadata: %w", err) + return npmVersionData{}, false, fmt.Errorf("parsing npm metadata: %w", err) } - return npmVersionData{entry: entry, published: published}, nil + return npmVersionData{entry: entry, published: published}, res.fromCache, nil } func validateTarballURL(raw, registryURL string) (string, error) { diff --git a/internal/handler/npm_attestations.go b/internal/handler/npm_attestations.go index 02c57513..cc840030 100644 --- a/internal/handler/npm_attestations.go +++ b/internal/handler/npm_attestations.go @@ -41,7 +41,7 @@ func (h *NPMHandler) handleAttestations(w http.ResponseWriter, r *http.Request) registryURL, cacheKey := h.sourceFor(packageName) metadata := sync.OnceValues(func() (npmVersionData, error) { - return h.versionData(r.Context(), registryURL+"/"+url.PathEscape(packageName), cacheKey, version) + return h.versionData(r.Context(), packageName, registryURL+"/"+url.PathEscape(packageName), cacheKey, version) }) inCooldown, err := h.versionInCooldown(packageName, version, metadata) diff --git a/internal/handler/npm_tarball_test.go b/internal/handler/npm_tarball_test.go index fe1508d8..d844dd55 100644 --- a/internal/handler/npm_tarball_test.go +++ b/internal/handler/npm_tarball_test.go @@ -201,3 +201,60 @@ func TestNPMTarballRejectsUnsafeMetadataURL(t *testing.T) { }) } } + +// TestNPMTarballCooldownRefetchesStalePackument checks that a tarball for a +// version the cached packument does not list yet is decided on the packument +// upstream serves now, not let through for want of a publish time. +func TestNPMTarballCooldownRefetchesStalePackument(t *testing.T) { + for _, tc := range []struct { + name string + published time.Time + down bool + want int + }{ + {"recent", time.Now().Add(-time.Hour), false, http.StatusNotFound}, + // Without upstream the version can't be checked, so it isn't served. + {"upstream down", time.Now().Add(-time.Hour), true, http.StatusBadGateway}, + // The refetched entry's integrity is checked, so a tarball that + // doesn't match it is refused rather than served unverified. + {"old", time.Now().Add(-30 * 24 * time.Hour), false, http.StatusBadGateway}, + } { + t.Run(tc.name, func(t *testing.T) { + var released atomic.Bool + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + old := time.Now().Add(-60 * 24 * time.Hour).Format(time.RFC3339) + if released.Load() && tc.down { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + if !released.Load() { + _, _ = fmt.Fprintf(w, `{"name":"widget","time":{"1.0.0":%q},"versions":{"1.0.0":{"dist":{}}}}`, old) + return + } + _, _ = fmt.Fprintf(w, `{"name":"widget","time":{"1.0.0":%q,"2.0.0":%q},"versions":{"1.0.0":{"dist":{}},"2.0.0":{"dist":{"integrity":%q}}}}`, + old, tc.published.Format(time.RFC3339), sha512SRI("package")) + })) + t.Cleanup(upstream.Close) + p, _, _, fetcher := setupTestProxy(t) + p.CacheMetadata = true + p.MetadataTTL = time.Hour + p.Cooldown = &cooldown.Config{Default: "7d"} + p.HTTPClient = upstream.Client() + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("tampered"))} + h := NewNPMHandler(p, "http://proxy.test", upstream.URL) + + metadata := httptest.NewRecorder() + h.Routes().ServeHTTP(metadata, httptest.NewRequest(http.MethodGet, "/widget", nil)) + if metadata.Code != http.StatusOK { + t.Fatalf("metadata status = %d: %s", metadata.Code, metadata.Body.String()) + } + released.Store(true) + + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/widget/-/widget-2.0.0.tgz", nil)) + if response.Code != tc.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tc.want, response.Body.String()) + } + }) + } +} diff --git a/internal/handler/pub.go b/internal/handler/pub.go index 31e00d8b..5a6d426c 100644 --- a/internal/handler/pub.go +++ b/internal/handler/pub.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/Masterminds/semver/v3" "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/retention" ) @@ -293,32 +294,47 @@ func (h *PubHandler) shouldFilterVersion(packagePURL, name, version string, vmap return false } -// updateLatestVersion updates the latest field if the current latest version -// was removed by cooldown filtering. +// updateLatestVersion points latest at the kept entry for its version, so +// its archive_url is the proxy's too. When cooldown removed that version, +// latest becomes the newest stable version left, or the newest version left +// if all are prereleases, and is dropped when none is left. func (h *PubHandler) updateLatestVersion(metadata map[string]any, filtered []any) { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - return - } - latest, ok := metadata["latest"].(map[string]any) if !ok { return } + latestVer, _ := latest["version"].(string) - latestVer, ok := latest["version"].(string) - if !ok { - return - } - + var newest, newestStable *semver.Version + var newestEntry, newestStableEntry any for _, vdata := range filtered { - if vmap, ok := vdata.(map[string]any); ok { - if vmap["version"] == latestVer { - return - } + vmap, ok := vdata.(map[string]any) + if !ok { + continue + } + version, _ := vmap["version"].(string) + if version == latestVer { + latest["archive_url"] = vmap["archive_url"] + return + } + parsed, err := semver.NewVersion(version) + if err != nil { + continue + } + if newest == nil || parsed.GreaterThan(newest) { + newest, newestEntry = parsed, vdata + } + if parsed.Prerelease() == "" && (newestStable == nil || parsed.GreaterThan(newestStable)) { + newestStable, newestStableEntry = parsed, vdata } } - if len(filtered) > 0 { - metadata["latest"] = filtered[len(filtered)-1] + switch { + case newestStableEntry != nil: + metadata["latest"] = newestStableEntry + case newestEntry != nil: + metadata["latest"] = newestEntry + default: + delete(metadata, "latest") } } diff --git a/internal/handler/pub_test.go b/internal/handler/pub_test.go index 29526e01..5311e25e 100644 --- a/internal/handler/pub_test.go +++ b/internal/handler/pub_test.go @@ -206,3 +206,58 @@ func TestPubDownloadServesCachedArchive(t *testing.T) { t.Errorf("body = %q, want %q", got, "cached archive") } } + +// TestPubRewriteMetadataLatest checks that latest points at the proxy and +// never names a version cooldown holds back, and that a held back latest is +// replaced with the newest stable version left. +func TestPubRewriteMetadataLatest(t *testing.T) { + old := time.Now().Add(-10 * 24 * time.Hour).Format(time.RFC3339) + recent := time.Now().Add(-time.Hour).Format(time.RFC3339) + entry := func(version, published string) string { + return `{"version":"` + version + `","published":"` + published + `","archive_url":"https://pub.dev/packages/demo/versions/` + version + `.tar.gz"}` + } + for _, tc := range []struct { + name, latest, versions string + cooldown bool + want, wantURL string + }{ + {"kept", entry("1.0.0", old), entry("1.0.0", old), false, "1.0.0", + "http://localhost:8080/pub/packages/demo/versions/1.0.0.tar.gz"}, + {"stable over prerelease", entry("2.0.0", recent), + entry("1.0.0", old) + "," + entry("1.1.0", old) + "," + entry("2.0.0-dev.1", old) + "," + entry("2.0.0", recent), true, "1.1.0", + "http://localhost:8080/pub/packages/demo/versions/1.1.0.tar.gz"}, + {"only prereleases", entry("2.0.0", recent), entry("2.0.0-dev.1", old) + "," + entry("2.0.0", recent), true, "2.0.0-dev.1", + "http://localhost:8080/pub/packages/demo/versions/2.0.0-dev.1.tar.gz"}, + {"all held back", entry("2.0.0", recent), entry("2.0.0", recent), true, "", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + proxy := &Proxy{Logger: slog.Default()} + if tc.cooldown { + proxy.Cooldown = &cooldown.Config{Default: "3d"} + } + h := &PubHandler{proxy: proxy, proxyURL: "http://localhost:8080"} + output, err := h.rewriteMetadata("demo", []byte(`{"name":"demo","latest":`+tc.latest+`,"versions":[`+tc.versions+`]}`)) + if err != nil { + t.Fatal(err) + } + var result struct { + Latest *struct { + Version string `json:"version"` + ArchiveURL string `json:"archive_url"` + } `json:"latest"` + } + if err := json.Unmarshal(output, &result); err != nil { + t.Fatal(err) + } + if tc.want == "" { + if result.Latest != nil { + t.Errorf("latest = %+v, want none", *result.Latest) + } + return + } + if result.Latest == nil || result.Latest.Version != tc.want || result.Latest.ArchiveURL != tc.wantURL { + t.Errorf("latest = %+v, want %s at %s", result.Latest, tc.want, tc.wantURL) + } + }) + } +} diff --git a/internal/handler/pypi.go b/internal/handler/pypi.go index ebb30828..ac4315f6 100644 --- a/internal/handler/pypi.go +++ b/internal/handler/pypi.go @@ -467,6 +467,19 @@ func (h *PyPIHandler) rewriteSimpleJSON(body []byte, filteredVersions map[string } metadata["files"] = rewrittenFiles + + // The versions list (PEP 700) loses the filtered versions too, whose + // files were all left out above. + if versions, ok := metadata["versions"].([]any); ok && len(filteredVersions) > 0 { + keptVersions := make([]any, 0, len(versions)) + for _, v := range versions { + if version, ok := v.(string); ok && filteredVersions[version] { + continue + } + keptVersions = append(keptVersions, v) + } + metadata["versions"] = keptVersions + } return json.Marshal(metadata) } diff --git a/internal/handler/pypi_test.go b/internal/handler/pypi_test.go index dd599741..f38492db 100644 --- a/internal/handler/pypi_test.go +++ b/internal/handler/pypi_test.go @@ -276,6 +276,32 @@ func TestPyPISimpleJSONCooldown(t *testing.T) { } } +// TestPyPISimpleJSONFiltersVersions checks that the PEP 700 versions list +// loses a version whose files were all filtered out, and keeps one that +// still has files. +func TestPyPISimpleJSONFiltersVersions(t *testing.T) { + h := &PyPIHandler{proxy: &Proxy{Logger: slog.Default()}, proxyURL: "http://proxy.test"} + body := `{"meta":{"api-version":"1.1"},"name":"ruff","versions":["1.0.0","2.0.0","3.0.0"],"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"},` + + `{"filename":"ruff-2.0.0-py3-none-any.whl","url":"https://files.pythonhosted.org/packages/ef/ruff-2.0.0-py3-none-any.whl"}` + + `]}` + out, err := h.rewriteSimpleJSON([]byte(body), map[string]bool{"2.0.0": true, "3.0.0": true}) + if err != nil { + t.Fatal(err) + } + var result struct { + Versions []string `json:"versions"` + } + if err := json.Unmarshal(out, &result); err != nil { + t.Fatal(err) + } + // 3.0.0 has no files to filter, but it is still held back. + if strings.Join(result.Versions, ",") != "1.0.0" { + t.Errorf("versions = %v, want [1.0.0]", result.Versions) + } +} + func TestPyPIParseFilename(t *testing.T) { h := &PyPIHandler{proxy: &Proxy{Logger: slog.Default()}} diff --git a/internal/metadata/npm/metadata.go b/internal/metadata/npm/metadata.go index 3a434c58..7fc4a40b 100644 --- a/internal/metadata/npm/metadata.go +++ b/internal/metadata/npm/metadata.go @@ -402,7 +402,7 @@ func (h *transformer) tarballsRewritten(count int) { } // applyCooldownFiltering removes versions that are too recently published, -// and updates dist-tags.latest if the current latest was filtered out. +// and updates the dist-tags that named them. func (h *transformer) applyCooldownFiltering(metadata map[string]any, versions map[string]any, packageName string) { if h.Cooldown == nil || !h.Cooldown.Enabled() { return @@ -415,6 +415,7 @@ func (h *transformer) applyCooldownFiltering(metadata map[string]any, versions m packagePURL := packageurl.MakeString("npm", packageName, "") + withheld := map[string]bool{} for version := range versions { publishedStr, ok := timeMap[version].(string) if !ok { @@ -430,20 +431,29 @@ func (h *transformer) applyCooldownFiltering(metadata map[string]any, versions m } delete(versions, version) delete(timeMap, version) + withheld[version] = true } } - h.updateDistTagsLatest(metadata, versions, timeMap) + h.updateDistTags(metadata, versions, timeMap, withheld) } -// updateDistTagsLatest updates the dist-tags.latest field if the current latest -// version was removed by cooldown filtering. -func (h *transformer) updateDistTagsLatest(metadata, versions, timeMap map[string]any) { +// updateDistTags moves dist-tags.latest to the newest remaining version if +// the current latest was removed by cooldown filtering, and, as the denylist +// does, drops other tags that name a withheld version. +func (h *transformer) updateDistTags(metadata, versions, timeMap map[string]any, withheld map[string]bool) { distTags, ok := metadata["dist-tags"].(map[string]any) if !ok { return } + for tag, value := range distTags { + version, _ := value.(string) + if tag != "latest" && withheld[version] { + delete(distTags, tag) + } + } + latest, ok := distTags["latest"].(string) if !ok { return @@ -455,6 +465,8 @@ func (h *transformer) updateDistTagsLatest(metadata, versions, timeMap map[strin if newLatest := h.findNewestVersion(versions, timeMap); newLatest != "" { distTags["latest"] = newLatest + } else if withheld[latest] { + delete(distTags, "latest") } } diff --git a/internal/metadata/npm/metadata_test.go b/internal/metadata/npm/metadata_test.go index 28e96691..2748ed34 100644 --- a/internal/metadata/npm/metadata_test.go +++ b/internal/metadata/npm/metadata_test.go @@ -194,3 +194,39 @@ func TestNPMRewriteAttestationsURL(t *testing.T) { } } } + +// TestNPMRewriteCooldownDropsTags checks that cooldown, like the denylist, +// drops a tag other than latest that names a version it withholds, in both +// the in-memory and the streamed rewrite. +func TestNPMRewriteCooldownDropsTags(t *testing.T) { + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + ts := func(age time.Duration) string { return now.Add(-age).UTC().Format(time.RFC3339) } + options := Options{ProxyURL: "http://proxy", EvaluatedAt: now, Cooldown: &cooldown.Config{Default: "3d"}} + input := `{"name":"demo",` + + `"dist-tags":{"latest":"3.0.0","next":"3.0.0","legacy":"1.0.0"},` + + `"versions":{"1.0.0":{},"2.0.0":{},"3.0.0":{}},` + + `"time":{"1.0.0":"` + ts(900*time.Hour) + `","2.0.0":"` + ts(800*time.Hour) + `","3.0.0":"` + ts(time.Hour) + `"}}` + + rewrite := map[string]func() ([]byte, []string, error){ + "memory": func() ([]byte, []string, error) { return Rewrite("demo", []byte(input), options) }, + "stream": func() ([]byte, []string, error) { return rewriteStream("demo", []byte(input), options) }, + } + for name, run := range rewrite { + t.Run(name, func(t *testing.T) { + out, _, err := run() + if err != nil { + t.Fatal(err) + } + var doc struct { + DistTags map[string]string `json:"dist-tags"` + } + if err := json.Unmarshal(out, &doc); err != nil { + t.Fatalf("output is not JSON: %v\n%s", err, out) + } + want := map[string]string{"latest": "2.0.0", "legacy": "1.0.0"} + if fmt.Sprint(doc.DistTags) != fmt.Sprint(want) { + t.Errorf("dist-tags = %v, want %v", doc.DistTags, want) + } + }) + } +} From 4f7a8b1b089fc5abd5ff4b03ea63d2bac8378ce9 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 08:01:11 +0100 Subject: [PATCH 2/6] Read Conda timestamps in seconds or milliseconds as conda does The Conda handler divided every repodata timestamp by 1000, assuming milliseconds. Some entries (a few in conda-forge noarch) use seconds, so their publish time came out in 1970 and they were never held back by cooldown. Read a timestamp as milliseconds only when it is above 253402300799, as conda does, in one helper used by the repodata filter, the shard filter and the download index. --- docs/configuration.md | 2 +- internal/handler/conda.go | 22 ++++++++++++++----- internal/handler/conda_test.go | 40 ++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 93de4100..e880230c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1232,7 +1232,7 @@ Maven caveats: Helm uses each release's `created` time from the repository's `index.yaml`. Releases in cooldown are left out of the index the proxy serves, and their archives aren't served; a release without `created` is let through. Charts pulled over OCI have no publish time and aren't held back. -Cargo, RubyGems, Hex, pub.dev, Conda and Helm check artifact downloads against cooldown, including files cached before the policy changed. When a publish-time lookup fails, an already-cached file is still served; an uncached download fails with 502, or 503 with `Retry-After` when the rate-limit gate holds the request. Conda checks timestamps per file because builds of one version can have different publish times. +Cargo, RubyGems, Hex, pub.dev, Conda and Helm check artifact downloads against cooldown, including files cached before the policy changed. When a publish-time lookup fails, an already-cached file is still served; an uncached download fails with 502, or 503 with `Retry-After` when the rate-limit gate holds the request. Conda checks timestamps per file because builds of one version can have different publish times. As in conda, a repodata `timestamp` is read as milliseconds when it is above 253402300799 (the end of year 9999 in seconds) and as seconds otherwise, since some older entries use seconds. Conda repodata and Helm indexes are parsed once per cached document for download checks. With `cache_metadata` off, the parsed index is kept for one minute, including the fetch, so an install can reuse it across files. Conda streams repodata when its compressed copy exceeds `metadata_max_size`, under the same decompressed size bound as the metadata filter. diff --git a/internal/handler/conda.go b/internal/handler/conda.go index 1c14980a..f593820c 100644 --- a/internal/handler/conda.go +++ b/internal/handler/conda.go @@ -432,8 +432,18 @@ func (c *countingWriter) Write(p []byte) (int, error) { return n, err } -// condaTimestampDivisor converts Conda's millisecond timestamps to seconds. -const condaTimestampDivisor = 1000 +// condaMaxSecondsTimestamp is the last second of year 9999. Like conda, a +// larger repodata timestamp is read as milliseconds and a smaller one as +// seconds, which some older entries use. +const condaMaxSecondsTimestamp = 253402300799 + +// condaTime returns the time a repodata timestamp names. +func condaTime(timestamp float64) time.Time { + if timestamp > condaMaxSecondsTimestamp { + return time.UnixMilli(int64(timestamp)).Truncate(time.Second) + } + return time.Unix(int64(timestamp), 0) +} const ( repodataWriteBuffer = 64 << 10 @@ -503,7 +513,7 @@ func (f *repodataFilter) inCooldown(filename string, raw []byte) bool { if entry.Name == "" || entry.Timestamp == 0 { return false } - publishedAt := time.Unix(int64(entry.Timestamp)/condaTimestampDivisor, 0) + publishedAt := condaTime(entry.Timestamp) decision := f.h.proxy.cooldown().Evaluate("conda", canonicalPackagePURL("conda", entry.Name), publishedAt, f.now) if decision.Allowed { return false @@ -520,14 +530,14 @@ func (f *repodataFilter) inCooldown(filename string, raw []byte) bool { return true } -// recordInCooldown reports whether a package record with a millisecond +// recordInCooldown reports whether a package record with a repodata // timestamp was published too recently. Records without a name or // timestamp are kept. func (h *CondaHandler) recordInCooldown(filename, name, version string, timestamp float64) bool { if name == "" || timestamp == 0 { return false } - publishedAt := time.Unix(int64(timestamp)/condaTimestampDivisor, 0) + publishedAt := condaTime(timestamp) if h.proxy.cooldown().IsAllowed("conda", canonicalPackagePURL("conda", name), publishedAt) { return false } @@ -620,7 +630,7 @@ func readRepodataIndex(r io.Reader) (map[string]time.Time, error) { } var e repodataEntry if json.Unmarshal(raw, &e) == nil && e.Timestamp != 0 { - index[entry.Key] = time.Unix(int64(e.Timestamp)/condaTimestampDivisor, 0) + index[entry.Key] = condaTime(e.Timestamp) } return nil }) diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 0c4c52af..63e518e4 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -3,9 +3,13 @@ package handler import ( "bytes" "encoding/json" + "fmt" "log/slog" + "maps" "net/http" "net/http/httptest" + "slices" + "strings" "testing" "time" @@ -133,6 +137,42 @@ func TestCondaCooldownFiltering(t *testing.T) { } } +// TestCondaCooldownSecondTimestamps checks that a timestamp in seconds, +// which some repodata entries carry, is read as seconds, as conda does, in +// both the filter and the download index. +func TestCondaCooldownSecondTimestamps(t *testing.T) { + recent := time.Now().Add(-time.Hour) + body := fmt.Sprintf(`{"packages":{"seconds-1.0-0.tar.bz2":{"name":"seconds","version":"1.0","timestamp":%d},`+ + `"millis-1.0-0.tar.bz2":{"name":"millis","version":"1.0","timestamp":%d}}}`, recent.Unix(), recent.UnixMilli()) + + proxy := testProxy() + proxy.Cooldown = &cooldown.Config{Default: "3d"} + h := &CondaHandler{proxy: proxy, proxyURL: "http://localhost:8080"} + filtered, err := filterRepodataBytes(h, []byte(body)) + if err != nil { + t.Fatal(err) + } + var result struct { + Packages map[string]json.RawMessage `json:"packages"` + } + if err := json.Unmarshal(filtered, &result); err != nil { + t.Fatal(err) + } + if len(result.Packages) != 0 { + t.Errorf("packages = %v, want both left out", slices.Sorted(maps.Keys(result.Packages))) + } + + index, err := readRepodataIndex(strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + for filename, published := range index { + if published.Unix() != recent.Unix() { + t.Errorf("%s published at %v, want %v", filename, published, recent) + } + } +} + func TestCondaCooldownFilteringWithPackageOverride(t *testing.T) { now := time.Now() recentTimestamp := float64(now.Add(-2 * time.Hour).UnixMilli()) From a288afab3283c61277841ae8ce39abe305eef39e Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 08:04:09 +0100 Subject: [PATCH 3/6] Match denylist entries for distro packages as handlers name them The denylist kept the PURL namespace, but the Debian and RPM handlers build pkg:deb/{name} and pkg:rpm/{name} without a vendor, while Alpine and Arch build pkg:apk/alpine/{name} and pkg:alpm/arch/{name} with one. So standard entries such as pkg:deb/debian/curl@... or pkg:rpm/fedora/curl@..., and pkg:apk/busybox@... or pkg:alpm/glibc@..., never matched. Drop the namespace of distro types on both sides, as retention and the inventory already do, using packageurl's list of those types. Debian and RPM handlers read versions from filenames, which leave out the epoch, so an entry such as pkg:deb/foo@1:2.3-1 never matched foo_2.3-1_amd64.deb. An entry with an epoch now also covers the version without it. --- docs/configuration.md | 7 ++++++ internal/denylist/denylist.go | 16 +++++++++++++- internal/denylist/denylist_test.go | 35 ++++++++++++++++++++++++++++++ internal/packageurl/handler.go | 6 +++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index e880230c..5acbab90 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1270,6 +1270,13 @@ Names and versions are compared the way each ecosystem treats them: - PyPI names ignore case, and runs of `-`, `_` and `.` are equivalent (PEP 503), so `pkg:pypi/zope.interface@6.0` also covers `zope_interface-6.0-*.whl`. - NuGet ids and versions ignore case. +- Debian, RPM, Alpine, Arch and FreeBSD entries ignore the distribution + namespace: `pkg:deb/debian/curl@8.5.0-2` and `pkg:deb/curl@8.5.0-2` are the + same entry. +- Debian and RPM filenames leave out the epoch, so an entry with one also + covers the version without it: `pkg:deb/foo@1:2.3-1` denies + `foo_2.3-1_amd64.deb`. An entry without an epoch only matches a version + without one. - A RubyGems entry also covers that version's platform gems: `pkg:gem/nokogiri@1.16.0` denies `nokogiri-1.16.0-x86_64-linux.gem`. - Composer versions ignore a leading `v`, which Packagist keeps from the tag: diff --git a/internal/denylist/denylist.go b/internal/denylist/denylist.go index 76f2de5f..eea8ee39 100644 --- a/internal/denylist/denylist.go +++ b/internal/denylist/denylist.go @@ -40,6 +40,13 @@ func New(packages []string) (*Policy, error) { p.packages[key] = make(map[string]bool) } p.packages[key][id.version] = true + // Debian and RPM filenames leave the epoch out, so an entry with + // one also matches the version without it. + if pkg.Type == "deb" || pkg.Type == "rpm" { + if _, version, ok := strings.Cut(id.version, ":"); ok && version != "" { + p.packages[key][version] = true + } + } } return p, nil } @@ -128,6 +135,9 @@ func genericSwiftRegistry(pkg *purl.PURL) (scope, name string, ok bool) { // identify compares PURLs in purl's comparison form. The generic // swift-registry spelling is the proxy's own, so it is mapped here first. +// The namespace of a distro type names the distribution, which handlers +// include for some types and not others, so it is dropped as retention and +// the inventory do. func identify(pkg *purl.PURL) identity { if scope, name, ok := genericSwiftRegistry(pkg); ok { pkg = purl.New("swift", scope, name, pkg.Version, nil) @@ -138,6 +148,10 @@ func identify(pkg *purl.PURL) identity { // An entry without a channel matches every channel. channel = c.Qualifier("channel") } - key := purl.New(c.Type, c.Namespace, c.Name, "", nil).String() + namespace := c.Namespace + if packageurl.DistroType(c.Type) { + namespace = "" + } + key := purl.New(c.Type, namespace, c.Name, "", nil).String() return identity{key: packageurl.PolicyPackageKey(key), channel: channel, version: c.Version} } diff --git a/internal/denylist/denylist_test.go b/internal/denylist/denylist_test.go index a1106088..03edc993 100644 --- a/internal/denylist/denylist_test.go +++ b/internal/denylist/denylist_test.go @@ -90,3 +90,38 @@ func TestEcosystemSpellings(t *testing.T) { t.Error("pypi versions not found under another spelling") } } + +// TestDistroNamespaces checks that the distribution namespace of a distro +// package is ignored on both sides, as handlers build some of these PURLs +// without one and some with one. +func TestDistroNamespaces(t *testing.T) { + p, err := New([]string{"pkg:deb/debian/curl@8.5.0-2", "pkg:rpm/fedora/curl@8.6.0-1.fc40", "pkg:apk/busybox@1.36.1-r15", "pkg:alpm/glibc@2.39-1"}) + if err != nil { + t.Fatal(err) + } + for _, value := range []string{"pkg:deb/curl@8.5.0-2", "pkg:rpm/curl@8.6.0-1.fc40", "pkg:apk/alpine/busybox@1.36.1-r15", "pkg:alpm/arch/glibc@2.39-1"} { + if !p.Denied(value) { + t.Errorf("not denied: %s", value) + } + } + if p.Denied("pkg:deb/curl@8.5.0-3") { + t.Error("other version denied") + } +} + +// TestDistroEpochs checks that a deb or rpm entry with an epoch matches the +// version as it appears in a filename, which leaves the epoch out. +func TestDistroEpochs(t *testing.T) { + p, err := New([]string{"pkg:deb/foo@1:2.3-1", "pkg:rpm/bar@2:1.0-1.fc40"}) + if err != nil { + t.Fatal(err) + } + for _, value := range []string{"pkg:deb/foo@2.3-1", "pkg:deb/foo@1:2.3-1", "pkg:rpm/bar@1.0-1.fc40"} { + if !p.Denied(value) { + t.Errorf("not denied: %s", value) + } + } + if p.Denied("pkg:deb/foo@2.3-2") { + t.Error("other version denied") + } +} diff --git a/internal/packageurl/handler.go b/internal/packageurl/handler.go index ee3005ef..5edcf801 100644 --- a/internal/packageurl/handler.go +++ b/internal/packageurl/handler.go @@ -8,6 +8,12 @@ import ( // ("alpine", "debian"), not part of the package name the handlers store. var distroTypes = map[string]bool{"alpm": true, "apk": true, "bsd": true, "deb": true, "rpm": true} +// DistroType reports whether a PURL type's namespace names the distribution, +// so that handlers ignore it. +func DistroType(purlType string) bool { + return distroTypes[purlType] +} + // HandlerEcosystem returns the ecosystem name the proxy's handlers store // packages of a PURL type under. purl names gem and composer after their // registries, rubygems and packagist; the handlers use the type. From 7fdfd9908fbc35af46b3d6f6795aa6dfacb48f63 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 10:10:43 +0100 Subject: [PATCH 4/6] Refetch npm packuments only when a cached copy lacks the version Under cooldown, a tarball for a version the packument doesn't list is decided on a fresh fetch of the packument. That check had gaps: - A stale copy served because upstream failed counted as fetched, so the version went through with no publish time. It now triggers the refetch like a copy within its TTL. - A packument just stored from upstream counted as cached, so every request for an unlisted version fetched it twice. Only a stored row read within its TTL counts now. - A stored refetch that joined another caller's fetch could get that caller's stale fallback and take it as upstream's answer. The stored lookup now reports stale results, and a stale refetch is refused. - A refused refetch dropped the upstream error, so a rate-limit hold answered 502 instead of 503 with Retry-After, and a 404 was lost. Stale results now carry the upstream error, and the refusal wraps it. --- internal/handler/handler.go | 7 +- internal/handler/metadata_stored.go | 30 +++-- internal/handler/npm.go | 79 +++++++------ internal/handler/npm_tarball_test.go | 166 +++++++++++++++++++++++++++ 4 files changed, 230 insertions(+), 52 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index d316aabc..be818f25 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1829,7 +1829,7 @@ func (p *Proxy) staleMetadata(ctx context.Context, ecosystem, cacheKey string, v p.Logger.Info("serving metadata from cache", "ecosystem", ecosystem, "key", cacheKey) - return metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String, stale: true}.describedBy(entry) + return metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String, stale: true, upstreamErr: err}.describedBy(entry) } // metadataResult is what a metadata lookup hands back. A shared fetch gives @@ -1843,8 +1843,9 @@ type metadataResult struct { digest string err error // stale is set when the body is the cached copy served because upstream - // failed. - stale bool + // failed, and upstreamErr is that failure. + stale bool + upstreamErr error // fromCache is set when the body is a stored copy within its TTL, read // without asking upstream. fromCache bool diff --git a/internal/handler/metadata_stored.go b/internal/handler/metadata_stored.go index ffe524d9..fc16d43a 100644 --- a/internal/handler/metadata_stored.go +++ b/internal/handler/metadata_stored.go @@ -45,30 +45,26 @@ var errStoredMetadataChanged = errors.New("stored metadata changed while it was // It returns errMetadataNotStored when the proxy doesn't cache metadata or // storing the document failed, and the caller reads it into memory instead. func (p *Proxy) storedMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func(io.Reader) error) (*database.MetadataCacheEntry, error) { - return p.storedMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, validate, "") -} - -// restoreStoredMetadata is storedMetadataDocument for a document whose row -// gone describes but whose stored copy was missing when read: the row is not -// served or revalidated, within its TTL or not, and the document is fetched -// again. -func (p *Proxy) restoreStoredMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func(io.Reader) error, gone *database.MetadataCacheEntry) (*database.MetadataCacheEntry, error) { - return p.storedMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, validate, gone.ContentDigest.String) + res := p.storedMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, validate, "") + return res.entry, res.err } -// storedMetadata implements storedMetadataDocument. A row with the digest -// gone is treated as if there were none. -func (p *Proxy) storedMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func(io.Reader) error, gone string) (*database.MetadataCacheEntry, error) { +// storedMetadata implements storedMetadataDocument, and reports in the +// result whether the row was read from cache or served stale. A row with +// the digest gone, one whose stored copy was missing or out of date when +// read, is not served or revalidated, within its TTL or not, and the +// document is fetched again. +func (p *Proxy) storedMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func(io.Reader) error, gone string) metadataResult { if !p.CacheMetadata || p.DB == nil || p.Storage == nil || p.MetadataTTL <= 0 { - return nil, errMetadataNotStored + return metadataResult{err: errMetadataNotStored} } if containsPathTraversal(cacheKey) { - return nil, fmt.Errorf("invalid cache key: %q", cacheKey) + return metadataResult{err: fmt.Errorf("invalid cache key: %q", cacheKey)} } if entry, fresh := p.storedMetadataEntryState(ctx, ecosystem, cacheKey, gone); fresh { metrics.RecordMetadataCacheHit(ecosystem, int(entry.Size.Int64)) p.recordMetadataRead(entry) - return entry, nil + return metadataResult{entry: entry, fromCache: true} } p.recordMetadataCacheMiss(ecosystem) @@ -102,7 +98,7 @@ func (p *Proxy) storedMetadata(ctx context.Context, ecosystem, cacheKey, upstrea res = p.fetchStoredMetadata(fetchCtx, ecosystem, cacheKey, upstreamURL, accept, validate, entry) return res }) - return res.entry, res.err + return res } // storedMetadataEntryState reads the cache row of a document and reports @@ -157,7 +153,7 @@ func (p *Proxy) staleStoredMetadata(ecosystem, cacheKey string, entry *database. if !usableStoredMetadata(entry) { return metadataResult{err: fmt.Errorf("upstream failed and no usable cached metadata: %w", err)} } - return metadataResult{entry: entry, stale: true} + return metadataResult{entry: entry, stale: true, upstreamErr: err} } // storeUpstreamMetadata fetches a document and streams it into storage, and diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 0e9f32ac..7eef4240 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -344,7 +344,7 @@ func (h *NPMHandler) serveStreamedPackument(w http.ResponseWriter, r *http.Reque fullRequired := h.packumentValidator() != nil options := h.npmRewriteOptions(packageName, nil) var rewritten rewrittenMetadata - err, fetchErr := h.readStoredPackument(r.Context(), upstreamURL, accept, metadataCacheKey, false, func(entry *database.MetadataCacheEntry) (err error) { + err, fetchErr := h.readStoredPackument(r.Context(), upstreamURL, accept, metadataCacheKey, func(entry *database.MetadataCacheEntry) (err error) { rewritten, err = h.proxy.streamedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, entry, func(open func() (io.ReadCloser, error), out io.Writer) error { var scan *npm.Scan err := readStored(open, func(in io.Reader) (err error) { @@ -409,27 +409,24 @@ func (h *NPMHandler) writeFetchError(w http.ResponseWriter, err error) { // document can't be read as a stream. var errInMemory = errors.New("npm packument needs the in-memory path") -// storedPackument returns the cache row of the packument stored for cacheKey, -// fetching it into storage first when needed. It returns errInMemory when +// storedPackument returns the lookup of the packument stored for cacheKey, +// fetching it into storage first when needed. Its err is errInMemory when // metadata isn't cached in storage, and ErrUpstreamNotFound and upstream // errors as they come. // With refresh, a stored copy within its TTL is fetched again rather than // served. -func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool) (*database.MetadataCacheEntry, error) { +func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool) metadataResult { + var gone string if refresh && h.proxy.DB != nil { if row, _ := h.proxy.DB.GetMetadataCache("npm", cacheKey); usableStoredMetadata(row) { - entry, err := h.proxy.restoreStoredMetadata(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator(), row) - if errors.Is(err, errMetadataNotStored) { - return nil, errInMemory - } - return entry, err + gone = row.ContentDigest.String } } - entry, err := h.proxy.storedMetadataDocument(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator()) - if errors.Is(err, errMetadataNotStored) { - return nil, errInMemory + res := h.proxy.storedMetadata(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator(), gone) + if errors.Is(res.err, errMetadataNotStored) { + res.err = errInMemory } - return entry, err + return res } // readStoredPackument reads the packument stored for cacheKey with read, @@ -437,23 +434,31 @@ func (h *NPMHandler) storedPackument(ctx context.Context, packumentURL, accept, // or no longer has the digest its row records, the packument is fetched // again and read once more. fetchErr is what storedPackument returned when the packument // couldn't be fetched, and err is what read returned. -func (h *NPMHandler) readStoredPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool, read func(*database.MetadataCacheEntry) error) (err, fetchErr error) { - entry, fetchErr := h.storedPackument(ctx, packumentURL, accept, cacheKey, refresh) - if fetchErr != nil { - return nil, fetchErr +func (h *NPMHandler) readStoredPackument(ctx context.Context, packumentURL, accept, cacheKey string, read func(*database.MetadataCacheEntry) error) (err, fetchErr error) { + fetched, err := h.lookupStoredPackument(ctx, packumentURL, accept, cacheKey, false, read) + return err, fetched.err +} + +// lookupStoredPackument implements readStoredPackument. fetched is the +// lookup of the packument read, whose err is set when it couldn't be +// fetched. +func (h *NPMHandler) lookupStoredPackument(ctx context.Context, packumentURL, accept, cacheKey string, refresh bool, read func(*database.MetadataCacheEntry) error) (fetched metadataResult, err error) { + fetched = h.storedPackument(ctx, packumentURL, accept, cacheKey, refresh) + if fetched.err != nil { + return fetched, nil } - if err = read(entry); !errors.Is(err, storage.ErrNotFound) && !errors.Is(err, errStoredMetadataChanged) { - return err, nil + if err = read(fetched.entry); !errors.Is(err, storage.ErrNotFound) && !errors.Is(err, errStoredMetadataChanged) { + return fetched, err } h.proxy.Logger.Warn("stored npm metadata is missing or changed, fetching it again", "url", packumentURL, "error", err) - entry, fetchErr = h.proxy.restoreStoredMetadata(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator(), entry) - if errors.Is(fetchErr, errMetadataNotStored) { - return nil, errInMemory + fetched = h.proxy.storedMetadata(ctx, "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator(), fetched.entry.ContentDigest.String) + if errors.Is(fetched.err, errMetadataNotStored) { + fetched.err = errInMemory } - if fetchErr != nil { - return nil, fetchErr + if fetched.err != nil { + return fetched, nil } - return read(entry), nil + return fetched, read(fetched.entry) } // packumentStreamValidator is packumentValidator for a packument read from @@ -486,7 +491,7 @@ func (h *NPMHandler) streamedVersionMetadata(ctx context.Context, packageName, s fullRequired := h.packumentValidator() != nil options := h.npmRewriteOptions(packageName, nil) var m rewrittenMetadata - err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, false, func(entry *database.MetadataCacheEntry) (err error) { + err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, func(entry *database.MetadataCacheEntry) (err error) { m, err = h.proxy.streamedRewrite(ctx, "npm", h.proxyURL, metadataCacheKey, entry, func(open func() (io.ReadCloser, error), out io.Writer) error { var scan *npm.Scan err := readStored(open, func(in io.Reader) (err error) { @@ -910,19 +915,24 @@ func (h *NPMHandler) versionData(ctx context.Context, packageName, packumentURL, } // readVersionData implements versionData. fromCache reports whether the -// packument may have been a cached copy; refresh fetches it from upstream. +// packument was a cached copy, within its TTL or served stale; refresh +// fetches it from upstream. func (h *NPMHandler) readVersionData(ctx context.Context, packumentURL, cacheKey, version string, refresh bool) (data npmVersionData, fromCache bool, err error) { accept := h.packumentAccept() var found npm.VersionEntry - err, fetchErr := h.readStoredPackument(ctx, packumentURL, accept, cacheKey, refresh, func(entry *database.MetadataCacheEntry) error { + fetched, err := h.lookupStoredPackument(ctx, packumentURL, accept, cacheKey, refresh, func(entry *database.MetadataCacheEntry) error { return readStored(func() (io.ReadCloser, error) { return h.proxy.openStoredMetadata(ctx, entry) }, func(in io.Reader) (err error) { found, err = npm.LookupVersion(in, version) return err }) }) + fetchErr := fetched.err switch { case fetchErr == nil && err == nil && (!found.Abbreviated || h.packumentValidator() == nil): - return npmVersionData{entry: found.Entry, published: found.Published}, !refresh, nil + if refresh && fetched.stale { + return npmVersionData{}, false, errRefetchFailed(fetched) + } + return npmVersionData{entry: found.Entry, published: found.Published}, fetched.fromCache || fetched.stale, nil case fetchErr == nil && err != nil: h.proxy.Logger.Warn("failed to read stored npm metadata, reading it in memory", "url", packumentURL, "error", err) case fetchErr != nil && !errors.Is(fetchErr, errInMemory): @@ -938,16 +948,21 @@ func (h *NPMHandler) readVersionData(ctx context.Context, packumentURL, cacheKey if res.err != nil { return npmVersionData{}, false, res.err } - // The cached copy is what lacked the version, so it can't decide it. if refresh && res.stale { - return npmVersionData{}, false, errors.New("npm metadata could not be fetched again") + return npmVersionData{}, false, errRefetchFailed(res) } entry, _ := jsonscan.Lookup(res.body, "versions", version) published, _, err := jsonscan.LookupString(res.body, "time", version) if err != nil { return npmVersionData{}, false, fmt.Errorf("parsing npm metadata: %w", err) } - return npmVersionData{entry: entry, published: published}, res.fromCache, nil + return npmVersionData{entry: entry, published: published}, res.fromCache || res.stale, nil +} + +// errRefetchFailed is the error for a refetch that got only the stale copy +// back. That copy is what lacked the version, so it can't decide it. +func errRefetchFailed(res metadataResult) error { + return fmt.Errorf("npm metadata could not be fetched again: %w", res.upstreamErr) } func validateTarballURL(raw, registryURL string) (string, error) { diff --git a/internal/handler/npm_tarball_test.go b/internal/handler/npm_tarball_test.go index d844dd55..6bca0f6c 100644 --- a/internal/handler/npm_tarball_test.go +++ b/internal/handler/npm_tarball_test.go @@ -1,6 +1,7 @@ package handler import ( + "context" "encoding/json" "fmt" "io" @@ -13,6 +14,7 @@ import ( "time" "github.com/git-pkgs/cooldown" + registryclient "github.com/git-pkgs/registries/client" "github.com/git-pkgs/registries/fetch" ) @@ -258,3 +260,167 @@ func TestNPMTarballCooldownRefetchesStalePackument(t *testing.T) { }) } } + +// widgetPackument is a packument listing only widget 1.0.0, published long +// ago. +func widgetPackument() string { + old := time.Now().Add(-60 * 24 * time.Hour).Format(time.RFC3339) + return fmt.Sprintf(`{"name":"widget","time":{"1.0.0":%q},"versions":{"1.0.0":{"dist":{}}}}`, old) +} + +// cooldownNPMHandler returns an npm handler for upstream with cooldown on +// and a tarball fetcher that would serve anything asked of it. +func cooldownNPMHandler(t *testing.T, upstream *httptest.Server, ttl time.Duration) *NPMHandler { + t.Helper() + p, _, _, fetcher := setupTestProxy(t) + p.CacheMetadata = true + p.MetadataTTL = ttl + p.Cooldown = &cooldown.Config{Default: "7d"} + p.HTTPClient = upstream.Client() + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("package"))} + return NewNPMHandler(p, "http://proxy.test", upstream.URL) +} + +// TestNPMTarballCooldownStalePackument checks that a stale packument served +// because upstream failed doesn't let through a version it doesn't list, +// and that the upstream failure decides the response. +func TestNPMTarballCooldownStalePackument(t *testing.T) { + for _, tc := range []struct { + name string + status int + limited bool + want int + }{ + {"upstream down", http.StatusServiceUnavailable, false, http.StatusBadGateway}, + {"not found", http.StatusNotFound, false, http.StatusNotFound}, + {"rate limited", http.StatusTooManyRequests, true, http.StatusServiceUnavailable}, + } { + t.Run(tc.name, func(t *testing.T) { + var down atomic.Bool + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if down.Load() { + w.Header().Set("Retry-After", "60") + w.WriteHeader(tc.status) + return + } + _, _ = io.WriteString(w, widgetPackument()) + })) + t.Cleanup(upstream.Close) + // With no TTL the packument is read in memory and every read + // asks upstream. + h := cooldownNPMHandler(t, upstream, 0) + if tc.limited { + gate := registryclient.NewRateLimitGate(http.DefaultTransport) + h.proxy.HTTPClient = &http.Client{Transport: gate} + h.proxy.RateLimits = gate + } + + metadata := httptest.NewRecorder() + h.Routes().ServeHTTP(metadata, httptest.NewRequest(http.MethodGet, "/widget", nil)) + if metadata.Code != http.StatusOK { + t.Fatalf("metadata status = %d: %s", metadata.Code, metadata.Body.String()) + } + down.Store(true) + + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/widget/-/widget-2.0.0.tgz", nil)) + if response.Code != tc.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tc.want, response.Body.String()) + } + if tc.limited && response.Header().Get("Retry-After") == "" { + t.Error("no Retry-After on a rate-limited response") + } + }) + } +} + +// TestNPMTarballCooldownFetchesMissingVersionOnce checks that a packument +// just fetched from upstream isn't fetched again because it doesn't list the +// requested version. +func TestNPMTarballCooldownFetchesMissingVersionOnce(t *testing.T) { + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + _, _ = io.WriteString(w, widgetPackument()) + })) + t.Cleanup(upstream.Close) + h := cooldownNPMHandler(t, upstream, time.Hour) + + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/widget/-/widget-99.0.0.tgz", nil)) + if n := requests.Load(); n != 1 { + t.Errorf("upstream packument requests = %d, want 1", n) + } +} + +// TestNPMTarballCooldownRefreshIgnoresSharedStale checks that a refetch of a +// stored packument lacking the version, joining another caller's fetch that +// falls back to the stored copy, doesn't take that copy as upstream's answer. +func TestNPMTarballCooldownRefreshIgnoresSharedStale(t *testing.T) { + var blocked atomic.Bool + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !blocked.Load() { + _, _ = io.WriteString(w, widgetPackument()) + return + } + arrived <- struct{}{} + select { + case <-release: + case <-r.Context().Done(): + } + w.WriteHeader(http.StatusServiceUnavailable) + })) + t.Cleanup(upstream.Close) + h := cooldownNPMHandler(t, upstream, time.Hour) + p := h.proxy + + metadata := httptest.NewRecorder() + h.Routes().ServeHTTP(metadata, httptest.NewRequest(http.MethodGet, "/widget", nil)) + if metadata.Code != http.StatusOK { + t.Fatalf("metadata status = %d: %s", metadata.Code, metadata.Body.String()) + } + registryURL, cacheKey := h.sourceFor("widget") + packumentURL := registryURL + "/widget" + accept := h.packumentAccept() + row, err := p.DB.GetMetadataCache("npm", cacheKey) + if err != nil || row == nil { + t.Fatalf("no stored packument row: %v", err) + } + setFetchedAt := func(at time.Time) { + t.Helper() + if ok, err := p.DB.RefreshMetadataCache("npm", cacheKey, row.ContentDigest.String, at); err != nil || !ok { + t.Fatalf("setting fetched_at: %t, %v", ok, err) + } + } + + // Another caller revalidates the expired copy, and upstream holds it. + setFetchedAt(time.Now().Add(-2 * time.Hour)) + blocked.Store(true) + other := make(chan error, 1) + go func() { + _, err := p.storedMetadataDocument(context.Background(), "npm", cacheKey, packumentURL, accept, h.packumentStreamValidator()) + other <- err + }() + <-arrived + + // The tarball request reads the copy as within its TTL and refetches it, + // joining that fetch. + setFetchedAt(time.Now()) + response := httptest.NewRecorder() + done := make(chan struct{}) + go func() { + defer close(done) + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/widget/-/widget-2.0.0.tgz", nil)) + }() + waitForMetadataWaiters(t, p, metadataCoalesceKey("npm", cacheKey, packumentURL, accept, "", false)+"\x00stored", 1) + close(release) + <-done + if err := <-other; err != nil { + t.Fatalf("other caller: %v", err) + } + if response.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want %d: %s", response.Code, http.StatusBadGateway, response.Body.String()) + } +} From fede2463f7a9fa54ae5419a7a34a989575f67c83 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 10:10:43 +0100 Subject: [PATCH 5/6] Describe conda shard timestamps as condaTime reads them --- internal/handler/conda_shards.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/handler/conda_shards.go b/internal/handler/conda_shards.go index 01434e48..8269f1db 100644 --- a/internal/handler/conda_shards.go +++ b/internal/handler/conda_shards.go @@ -378,7 +378,8 @@ func rewriteCondaShardIndex(body []byte, indexURL string, mask [condaShardHashLe } // condaShardRecord holds the fields cooldown needs from a shard record. -// Timestamps are milliseconds, encoded as an integer or a float. +// Timestamps are an integer or a float, in seconds or milliseconds as +// condaTime reads them. type condaShardRecord struct { Name string `msgpack:"name"` Version string `msgpack:"version"` From 1b22f79be2d0021d2bdb41faf0419f7511e57e34 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 8 Nov 2026 17:23:11 +0000 Subject: [PATCH 6/6] Fail the conda timestamp test when the index is empty --- internal/handler/conda_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 63e518e4..f6c37c65 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -166,6 +166,9 @@ func TestCondaCooldownSecondTimestamps(t *testing.T) { if err != nil { t.Fatal(err) } + if len(index) != 2 { + t.Fatalf("index has %d entries, want 2", len(index)) + } for filename, published := range index { if published.Unix() != recent.Unix() { t.Errorf("%s published at %v, want %v", filename, published, recent)