From 743fbe50264660af73fe69536aebdd0e6ce37dba Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 23:28:13 +0200 Subject: [PATCH 1/2] Enable storage retention for hex Tarballs are stored under the name from the tarball filename, and purl keeps hex names as they are, so package rules use the default mapping with the name as published. The filename parser splits versions like 4.1.5-1 at the wrong hyphen and files them under a package like amqp_client-4.1.5, so package rules don't reach them; the docs row says so until the parser is fixed. Refs #306 --- docs/configuration.md | 2 +- internal/handler/hex.go | 10 ++ internal/server/retention_hex_test.go | 234 ++++++++++++++++++++++++++ 3 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 internal/server/retention_hex_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 97fb0f5b..0a67c47b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,7 +98,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `gem` | no | - | | `golang` | yes | `pkg:golang/github.com/stretchr/testify`, the module path as in `go.mod`; case doesn't matter. A major version like `/v2` (or `.v3` on gopkg.in) is a module of its own and needs its own rule. The `!`-escaped form from proxy URLs and the module cache (`github.com/!azure/...`) is refused. | | `helm` | yes | No package rules: charts are cached per repository and digest, not under the chart name. The `helm` rule and `default` cover charts from HTTP repositories; charts pulled over OCI fall under `oci`. | -| `hex` | no | - | +| `hex` | yes | `pkg:hex/phoenix`, the package name as published. Package rules don't reach versions like `4.1.5-1` or `0.1.0-dev-10` yet, which the proxy files under a wrong package name; the hex rule and `default` still cover them. | | `julia` | yes | No package rules: a julia PURL needs a `uuid` qualifier, which the cache doesn't store. The `julia` rule and `default` cover package tarballs, artifacts and registry snapshots alike. | | `maven` | yes | `pkg:maven/com.google.guava/guava`, group and artifact as published, case included. Gradle build cache entries are not covered; they use `gradle.build_cache.max_age`. | | `npm` | yes | `pkg:npm/lodash`, `pkg:npm/@babel/core`; the name has to match exactly, case included, so `pkg:npm/jsonstream` doesn't cover `JSONStream`. | diff --git a/internal/handler/hex.go b/internal/handler/hex.go index c475e5a9..adae0de0 100644 --- a/internal/handler/hex.go +++ b/internal/handler/hex.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/git-pkgs/proxy/internal/retention" "google.golang.org/protobuf/encoding/protowire" ) @@ -17,6 +18,15 @@ const ( hexUpstream = "https://repo.hex.pm" ) +// Tarballs are stored under pkg:hex/{name} with the name from the tarball +// filename, and purl keeps hex names as they are, so package rules use the +// default key mapping. parseTarballFilename splits at the last hyphen before +// a digit, so a version like 4.1.5-1 lands under a wrong package name that +// package rules don't reach. +func init() { + retention.Default.Register(retention.Spec{Key: "hex", CanonicalPackage: retention.DefaultCanonical("hex")}) +} + // HexHandler handles Hex.pm registry protocol requests. type HexHandler struct { proxy *Proxy diff --git a/internal/server/retention_hex_test.go b/internal/server/retention_hex_test.go new file mode 100644 index 00000000..4873dbbd --- /dev/null +++ b/internal/server/retention_hex_test.go @@ -0,0 +1,234 @@ +package server + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/database" + "github.com/git-pkgs/proxy/internal/handler" + "github.com/git-pkgs/proxy/internal/mirror" + "github.com/git-pkgs/proxy/internal/retention" + "github.com/git-pkgs/registries/fetch" +) + +const ( + hexPhoenixPath = "/tarballs/phoenix-1.7.10.tar" + hexPlugPath = "/tarballs/plug-1.15.3.tar" +) + +// hexRetentionUpstream serves package tarballs the way repo.hex.pm does, +// counting downloads. +func hexRetentionUpstream(t *testing.T) (*httptest.Server, *atomic.Int64) { + t.Helper() + var downloads atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case hexPhoenixPath, hexPlugPath: + downloads.Add(1) + _, _ = io.WriteString(w, "hex tarball bytes") + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + return srv, &downloads +} + +func newHexRetentionProxy(t *testing.T, upstream *httptest.Server) (*httptest.Server, *database.DB) { + t.Helper() + db, store := setupEvictionTest(t) + // The upstream is a plain-HTTP loopback server; the default fetcher + // refuses loopback, a stock client reaches it. + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + proxy.HTTPClient = upstream.Client() + h := handler.NewHexHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL) + srv := httptest.NewServer(h.Routes()) + t.Cleanup(srv.Close) + return srv, db +} + +func hexGet(t *testing.T, url string) { + t.Helper() + resp, err := http.Get(url) + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s = %d %s, want 200", url, resp.StatusCode, body) + } +} + +// hexArtifact returns the record of the tarball named filename. +func hexArtifact(t *testing.T, db *database.DB, filename string) database.Artifact { + t.Helper() + var art database.Artifact + if err := db.Get(&art, db.Rebind(`SELECT * FROM artifacts WHERE filename = ?`), filename); err != nil { + t.Fatalf("reading artifact %s: %v", filename, err) + } + return art +} + +func hexBackdate(t *testing.T, db *database.DB, filename string, fetched, accessed time.Time) { + t.Helper() + query := db.Rebind(`UPDATE artifacts SET fetched_at = ?, last_accessed_at = ? WHERE filename = ?`) + if _, err := db.Exec(query, fetched, accessed, filename); err != nil { + t.Fatalf("backdating %s: %v", filename, err) + } +} + +func TestHexRegistersForRetention(t *testing.T) { + if _, ok := retention.Default.Lookup("hex"); !ok { + t.Fatal("hex is not registered for retention") + } + if key, ok := retention.Default.KeyForDBEcosystem("hex"); !ok || key != "hex" { + t.Errorf("KeyForDBEcosystem(hex) = %q, %v, want hex, true", key, ok) + } +} + +// A tarball served from the cache stays while it is used, is evicted once +// its rule runs out, comes back from upstream on the next download, and +// stays after that even with the evicted copy's old access time on the +// record. +func TestHexRetentionEvictsAndRefetches(t *testing.T) { + upstream, downloads := hexRetentionUpstream(t) + srv, db := newHexRetentionProxy(t, upstream) + const tarball = "phoenix-1.7.10.tar" + + hexGet(t, srv.URL+hexPhoenixPath) + hexGet(t, srv.URL+hexPhoenixPath) + if got := downloads.Load(); got != 1 { + t.Fatalf("upstream downloads = %d, want 1", got) + } + if !hexArtifact(t, db, tarball).LastAccessedAt.Valid { + t.Fatal("cache hit did not record an access") + } + + rules := retention.Rules{Ecosystems: map[string]time.Duration{"hex": retentionDay}} + now := time.Now() + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if !hexArtifact(t, db, tarball).StoragePath.Valid { + t.Fatal("tarball used today was evicted") + } + + old := now.Add(-2 * retentionDay) + hexBackdate(t, db, tarball, old, old) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if hexArtifact(t, db, tarball).StoragePath.Valid { + t.Fatal("tarball unused for two days kept under a one-day rule") + } + + hexGet(t, srv.URL+hexPhoenixPath) + if got := downloads.Load(); got != 2 { + t.Errorf("upstream downloads = %d, want 2 after the eviction", got) + } + art := hexArtifact(t, db, tarball) + hexBackdate(t, db, tarball, art.FetchedAt.Time, now.Add(-30*retentionDay)) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + if !hexArtifact(t, db, tarball).StoragePath.Valid { + t.Error("refetched tarball evicted again by the next sweep") + } +} + +// A key with a namespace parses as a hex PURL but can never match a stored +// package, so it is refused at validation. A plain key is accepted. +func TestHexRetentionRefusesNamespacedKeys(t *testing.T) { + cfg := config.Default() + cfg.Storage.Retention.Packages = map[string]string{"pkg:hex/phoenix": "0"} + if _, err := cfg.RetentionRules(retention.Default); err != nil { + t.Fatalf("RetentionRules(pkg:hex/phoenix): %v", err) + } + cfg.Storage.Retention.Packages = map[string]string{"pkg:hex/acme/phoenix": "0"} + _, err := cfg.RetentionRules(retention.Default) + if err == nil || !strings.Contains(err.Error(), "does not match how hex packages are stored") { + t.Errorf("RetentionRules(pkg:hex/acme/phoenix) error = %v, want the key refused", err) + } +} + +// A package rule written as a hex PURL reaches the stored package. +func TestHexRetentionPackageRule(t *testing.T) { + upstream, _ := hexRetentionUpstream(t) + srv, db := newHexRetentionProxy(t, upstream) + const phoenixTarball, plugTarball = "phoenix-1.7.10.tar", "plug-1.15.3.tar" + hexGet(t, srv.URL+hexPhoenixPath) + hexGet(t, srv.URL+hexPlugPath) + old := time.Now().Add(-30 * retentionDay) + hexBackdate(t, db, phoenixTarball, old, old) + hexBackdate(t, db, plugTarball, old, old) + + cfg := config.Default() + cfg.Storage.Retention.Ecosystems = map[string]string{"hex": "1d"} + cfg.Storage.Retention.Packages = map[string]string{"pkg:hex/phoenix": "0"} + rules, err := cfg.RetentionRules(retention.Default) + if err != nil { + t.Fatalf("RetentionRules: %v", err) + } + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + + if !hexArtifact(t, db, phoenixTarball).StoragePath.Valid { + t.Error("phoenix evicted despite its never-evict rule") + } + if hexArtifact(t, db, plugTarball).StoragePath.Valid { + t.Error("plug kept past the hex rule") + } +} + +// hexRewriteTransport sends every request to the test upstream, so the +// mirror command's fixed repo.hex.pm URLs reach it. +type hexRewriteTransport struct { + target *url.URL + base http.RoundTripper +} + +func (r hexRewriteTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.URL.Scheme = r.target.Scheme + req.URL.Host = r.target.Host + req.Host = r.target.Host + return r.base.RoundTrip(req) +} + +// A package preloaded with the mirror command falls under the hex rule. +func TestHexRetentionCoversMirroredPackages(t *testing.T) { + upstream, downloads := hexRetentionUpstream(t) + target, err := url.Parse(upstream.URL) + if err != nil { + t.Fatal(err) + } + db, store := setupEvictionTest(t) + client := &http.Client{Transport: hexRewriteTransport{target: target, base: upstream.Client().Transport}} + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(client), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + + m := mirror.New(proxy, db, store, discardLogger(), 1) + progress, err := m.Run(context.Background(), &mirror.PURLSource{PURLs: []string{"pkg:hex/phoenix@1.7.10"}}) + if err != nil || progress.Failed != 0 || downloads.Load() != 1 { + t.Fatalf("mirror run = %+v, %v, downloads %d; want one successful download", progress, err, downloads.Load()) + } + + const tarball = "phoenix-1.7.10.tar" + if hexArtifact(t, db, tarball).LastAccessedAt.Valid { + t.Fatal("mirrored tarball has an access recorded before anyone downloaded it") + } + query := db.Rebind(`UPDATE artifacts SET fetched_at = ? WHERE filename = ?`) + if _, err := db.Exec(query, time.Now().Add(-30*retentionDay), tarball); err != nil { + t.Fatalf("backdating %s: %v", tarball, err) + } + rules := retention.Rules{Ecosystems: map[string]time.Duration{"hex": retentionDay}} + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + if hexArtifact(t, db, tarball).StoragePath.Valid { + t.Error("never-served mirrored tarball kept past the hex rule") + } +} From c3c120401c205ec0b3eaccaeb68920c1c6dd5d78 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 19:57:57 +0200 Subject: [PATCH 2/2] Fix hex tarballs whose version contains a hyphen parseTarballFilename split at the last hyphen before a digit, so amqp_client-4.1.5-1.tar was stored as package amqp_client-4.1.5. Hex names can't contain a hyphen, so it now splits at the first one. The docs caveat is gone. Refs #306 --- docs/configuration.md | 2 +- internal/handler/hex.go | 20 +++++++++----------- internal/handler/hex_test.go | 5 +++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 0a67c47b..2aa2a2c8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,7 +98,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `gem` | no | - | | `golang` | yes | `pkg:golang/github.com/stretchr/testify`, the module path as in `go.mod`; case doesn't matter. A major version like `/v2` (or `.v3` on gopkg.in) is a module of its own and needs its own rule. The `!`-escaped form from proxy URLs and the module cache (`github.com/!azure/...`) is refused. | | `helm` | yes | No package rules: charts are cached per repository and digest, not under the chart name. The `helm` rule and `default` cover charts from HTTP repositories; charts pulled over OCI fall under `oci`. | -| `hex` | yes | `pkg:hex/phoenix`, the package name as published. Package rules don't reach versions like `4.1.5-1` or `0.1.0-dev-10` yet, which the proxy files under a wrong package name; the hex rule and `default` still cover them. | +| `hex` | yes | `pkg:hex/phoenix`, the package name as published. | | `julia` | yes | No package rules: a julia PURL needs a `uuid` qualifier, which the cache doesn't store. The `julia` rule and `default` cover package tarballs, artifacts and registry snapshots alike. | | `maven` | yes | `pkg:maven/com.google.guava/guava`, group and artifact as published, case included. Gradle build cache entries are not covered; they use `gradle.build_cache.max_age`. | | `npm` | yes | `pkg:npm/lodash`, `pkg:npm/@babel/core`; the name has to match exactly, case included, so `pkg:npm/jsonstream` doesn't cover `JSONStream`. | diff --git a/internal/handler/hex.go b/internal/handler/hex.go index adae0de0..79be510b 100644 --- a/internal/handler/hex.go +++ b/internal/handler/hex.go @@ -20,9 +20,7 @@ const ( // Tarballs are stored under pkg:hex/{name} with the name from the tarball // filename, and purl keeps hex names as they are, so package rules use the -// default key mapping. parseTarballFilename splits at the last hyphen before -// a digit, so a version like 4.1.5-1 lands under a wrong package name that -// package rules don't reach. +// default key mapping. func init() { retention.Default.Register(retention.Spec{Key: "hex", CanonicalPackage: retention.DefaultCanonical("hex")}) } @@ -86,16 +84,16 @@ func (h *HexHandler) handleDownload(w http.ResponseWriter, r *http.Request) { // parseTarballFilename extracts name and version from a hex tarball filename. // e.g., "phoenix-1.7.10.tar" -> ("phoenix", "1.7.10") +// e.g., "amqp_client-4.1.5-1.tar" -> ("amqp_client", "4.1.5-1") +// +// Hex package names can't contain a hyphen, so the first one separates the +// name from a version that may carry hyphens of its own. func (h *HexHandler) parseTarballFilename(filename string) (name, version string) { - base := strings.TrimSuffix(filename, ".tar") - - // Find the last hyphen followed by a version number - for i := len(base) - 1; i >= 0; i-- { - if base[i] == '-' && i+1 < len(base) && base[i+1] >= '0' && base[i+1] <= '9' { - return base[:i], base[i+1:] - } + name, version, ok := strings.Cut(strings.TrimSuffix(filename, ".tar"), "-") + if !ok || name == "" || version == "" { + return "", "" } - return "", "" + return name, version } // hexAPIURL is the Hex HTTP API base URL for fetching package metadata with timestamps. diff --git a/internal/handler/hex_test.go b/internal/handler/hex_test.go index b02540a2..92120bd6 100644 --- a/internal/handler/hex_test.go +++ b/internal/handler/hex_test.go @@ -26,6 +26,11 @@ func TestHexParseTarballFilename(t *testing.T) { {"phoenix-1.7.10.tar", "phoenix", "1.7.10"}, {"ecto-3.11.0.tar", "ecto", "3.11.0"}, {"phoenix_live_view-0.20.1.tar", "phoenix_live_view", "0.20.1"}, + {"amqp_client-4.1.5-1.tar", "amqp_client", "4.1.5-1"}, + {"bandit-0.1.0-dev-10.tar", "bandit", "0.1.0-dev-10"}, + {"ex_doc-0.34.0-rc.1.tar", "ex_doc", "0.34.0-rc.1"}, + {"-1.0.0.tar", "", ""}, + {"phoenix-.tar", "", ""}, {"invalid", "", ""}, }