From 1c6cfae81360a94e70e8067dcbd0bd1bdf286246 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 09:57:43 +0200 Subject: [PATCH 01/14] Serve the Go checksum database from its own URL Everything under /go/sumdb/ was passed on to the module upstream. But proxy.golang.org doesn't serve the checksum database, so those requests always came back 404 and every client had to talk to sum.golang.org itself. The Go handler now serves /sumdb/{name}/ for the databases in upstream.go_sumdbs, which defaults to sum.golang.org. Lookups, latest and tiles come from the database through the metadata cache. Complete tiles never change, so once they're cached we don't ask again. Unknown names and paths that aren't part of the protocol get a 404 without going upstream. If the database answers 404 we pass on up to 4 KiB of its text, because the go command prints it and it tells you why a version is missing. For that the not-found error from the metadata fetch now carries the start of the body. Other handlers still just see ErrUpstreamNotFound. --- go.mod | 2 +- internal/config/config.go | 10 + internal/config/config_test.go | 40 +++ internal/handler/download_test.go | 1 - internal/handler/go.go | 13 +- internal/handler/go_sumdb.go | 173 ++++++++++++ internal/handler/go_sumdb_test.go | 425 ++++++++++++++++++++++++++++++ internal/handler/handler.go | 17 +- internal/server/go_sumdbs_test.go | 38 +++ internal/server/server.go | 1 + 10 files changed, 709 insertions(+), 11 deletions(-) create mode 100644 internal/handler/go_sumdb.go create mode 100644 internal/handler/go_sumdb_test.go create mode 100644 internal/server/go_sumdbs_test.go diff --git a/go.mod b/go.mod index af9984d2..052604e5 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( github.com/spdx/tools-golang v0.5.7 github.com/swaggo/swag v1.16.6 gocloud.dev v0.46.0 + golang.org/x/mod v0.41.0 golang.org/x/net v0.59.0 golang.org/x/sync v0.23.0 google.golang.org/protobuf v1.36.12 @@ -117,7 +118,6 @@ require ( go.opentelemetry.io/otel/trace v1.45.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/crypto v0.57.0 // indirect - golang.org/x/mod v0.41.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sys v0.48.0 // indirect golang.org/x/text v0.42.0 // indirect diff --git a/internal/config/config.go b/internal/config/config.go index b79a2463..46cda82a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -573,6 +573,13 @@ type UpstreamConfig struct { // Default: https://proxy.golang.org Go string `json:"go" yaml:"go"` + // GoSumDBs maps Go checksum database names to their base URLs, served at + // /go/sumdb/{name}/ so the go command reaches the database through + // GOPROXY. Unknown names get 404, which makes the go command contact the + // database directly. + // Default when empty: {"sum.golang.org": "https://sum.golang.org"}. + GoSumDBs map[string]string `json:"go_sumdbs" yaml:"go_sumdbs"` + // Hex is the upstream Hex repository URL. // Default: https://repo.hex.pm Hex string `json:"hex" yaml:"hex"` @@ -773,6 +780,9 @@ func (u *UpstreamConfig) Validate() error { if err := validateNamedUpstreams("upstream.arch", u.Arch); err != nil { return err } + if err := validateNamedUpstreams("upstream.go_sumdbs", u.GoSumDBs); err != nil { + return err + } if err := validateNamedUpstreams("upstream.oci", u.OCI); err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9bee2dca..88150ae4 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1612,6 +1612,46 @@ func TestValidateNamedUpstreams(t *testing.T) { } } +func TestValidateGoSumDBs(t *testing.T) { + tests := []struct { + name string + modify func(*Config) + wantErr bool + }{ + { + name: "valid Go checksum database", + modify: func(cfg *Config) { + cfg.Upstream.GoSumDBs = map[string]string{"sum.golang.org": "https://sum.golang.org"} + }, + }, + { + name: "Go checksum database name contains path separator", + modify: func(cfg *Config) { + cfg.Upstream.GoSumDBs = map[string]string{"sum.golang.org/x": "https://sum.golang.org"} + }, + wantErr: true, + }, + { + name: "Go checksum database URL is not absolute", + modify: func(cfg *Config) { + cfg.Upstream.GoSumDBs = map[string]string{"sum.golang.org": "sum.golang.org"} + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := Default() + tt.modify(cfg) + err := cfg.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %t", err, tt.wantErr) + } + }) + } +} + func TestValidateCacheArtifactsDisabled(t *testing.T) { tests := []struct { name string diff --git a/internal/handler/download_test.go b/internal/handler/download_test.go index 0df443d7..2797d421 100644 --- a/internal/handler/download_test.go +++ b/internal/handler/download_test.go @@ -330,7 +330,6 @@ func TestGoHandler_UpstreamProxy(t *testing.T) { "/golang.org/x/text/@v/v0.14.0.info", "/golang.org/x/text/@v/v0.14.0.mod", "/golang.org/x/text/@latest", - "/sumdb/sum.golang.org/lookup/golang.org/x/text@v0.14.0", } for _, path := range tests { diff --git a/internal/handler/go.go b/internal/handler/go.go index 353edbbc..6348a1ea 100644 --- a/internal/handler/go.go +++ b/internal/handler/go.go @@ -31,6 +31,7 @@ type GoHandler struct { proxy *Proxy upstreamURL string proxyURL string + sumDBs map[string]string } // NewGoHandler creates a new Go module proxy handler. @@ -39,6 +40,7 @@ func NewGoHandler(proxy *Proxy, proxyURL string) *GoHandler { proxy: proxy, upstreamURL: goUpstream, proxyURL: strings.TrimSuffix(proxyURL, "/"), + sumDBs: map[string]string{defaultGoSumDBName: defaultGoSumDBURL}, } } @@ -65,9 +67,9 @@ func (h *GoHandler) Routes() http.Handler { func (h *GoHandler) handleRequest(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/") - // Sumdb requests - proxy through - if strings.HasPrefix(path, "sumdb/") { - h.proxyUpstream(w, r) + // Checksum database requests go to the database, not the module upstream + if rest, ok := strings.CutPrefix(path, "sumdb/"); ok { + h.handleSumDB(w, r, rest) return } @@ -141,11 +143,6 @@ func (h *GoHandler) handleDownload(w http.ResponseWriter, r *http.Request, modul ServeArtifactRequest(w, r, result) } -// proxyUpstream forwards a request to proxy.golang.org without caching. -func (h *GoHandler) proxyUpstream(w http.ResponseWriter, r *http.Request) { - h.proxy.ProxyUpstream(w, r, h.upstreamURL+r.URL.Path, nil) -} - // proxyCached forwards a request with metadata caching. func (h *GoHandler) proxyCached(w http.ResponseWriter, r *http.Request, cacheKey string) { h.proxy.ProxyCached(w, r, h.upstreamURL+r.URL.Path, "golang", cacheKey, "*/*") diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go new file mode 100644 index 00000000..32665f62 --- /dev/null +++ b/internal/handler/go_sumdb.go @@ -0,0 +1,173 @@ +package handler + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "net/http" + "strconv" + "strings" + + "github.com/git-pkgs/proxy/internal/metrics" + "golang.org/x/mod/module" + "golang.org/x/mod/semver" + "golang.org/x/mod/sumdb/tlog" +) + +const ( + // defaultGoSumDBName is the checksum database the go command uses when + // GOSUMDB is not set. + defaultGoSumDBName = "sum.golang.org" + defaultGoSumDBURL = "https://sum.golang.org" + goEcosystem = "golang" +) + +// SetSumDBs sets the checksum databases served at /sumdb/{name}/, mapping +// each database name to its base URL. An empty map keeps the default, +// sum.golang.org. +func (h *GoHandler) SetSumDBs(sumDBs map[string]string) { + if len(sumDBs) == 0 { + h.sumDBs = map[string]string{defaultGoSumDBName: defaultGoSumDBURL} + return + } + h.sumDBs = make(map[string]string, len(sumDBs)) + for name, sumDBURL := range sumDBs { + h.sumDBs[name] = strings.TrimSuffix(sumDBURL, "/") + } +} + +// handleSumDB serves checksum database requests that the go command sends +// through GOPROXY as /sumdb/{name}/{path}. The module proxy upstream does not +// serve them, so they are fetched from the configured database itself. +// Unknown names get 404, which makes the go command contact the database +// directly. +func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest string) { + name, path, _ := strings.Cut(rest, "/") + sumDBURL, ok := h.sumDBs[name] + if !ok { + http.NotFound(w, r) + return + } + if path == "supported" { + w.WriteHeader(http.StatusOK) + return + } + immutable, valid := parseGoSumDBPath(path) + if !valid { + http.NotFound(w, r) + return + } + + cacheKey := goSumDBCacheKey(name, sumDBURL, path) + if immutable && h.serveCachedSumDBTile(w, r, cacheKey) { + return + } + h.proxySumDB(w, r, sumDBURL+"/"+path, cacheKey) +} + +// parseGoSumDBPath reports whether path is a request the checksum database +// protocol defines and whether its response can never change. Only complete +// tiles are immutable; partial tiles, latest and lookups follow the metadata +// TTL. +func parseGoSumDBPath(path string) (immutable, valid bool) { + if path == "latest" { + return false, true + } + if mod, ok := strings.CutPrefix(path, "lookup/"); ok { + return false, validGoSumDBLookup(mod) + } + tile, err := tlog.ParseTilePath(path) + if err != nil { + return false, false + } + return tile.W == 1< 0: + w.Header().Set(headerContentType, "text/plain; charset=utf-8") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write(notFound.body) + case errors.Is(err, ErrUpstreamNotFound): + http.Error(w, "not found", http.StatusNotFound) + default: + p.Logger.Error("checksum database fetch failed", "url", upstreamURL, "error", err) + http.Error(w, "failed to fetch from upstream", http.StatusBadGateway) + } + return + } + + p.writeMetadataCachedResponseWithEncoding(w, r, goEcosystem, cacheKey, body, contentType, contentEncoding) +} diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go new file mode 100644 index 00000000..a9ff1dfa --- /dev/null +++ b/internal/handler/go_sumdb_test.go @@ -0,0 +1,425 @@ +package handler + +import ( + "bytes" + "crypto/rand" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "golang.org/x/mod/sumdb" + "golang.org/x/mod/sumdb/note" +) + +const testSumDBName = "sum.example" + +// fakeSumDB is a checksum database upstream serving fixed files. Unknown +// paths get 404 with the body from notFound, or a default text. +type fakeSumDB struct { + server *httptest.Server + files map[string]string + notFound map[string]string + + mu sync.Mutex + hits map[string]int + down atomic.Bool +} + +func newFakeSumDB(t *testing.T, files map[string]string) *fakeSumDB { + t.Helper() + db := &fakeSumDB{files: files, notFound: map[string]string{}, hits: map[string]int{}} + db.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + db.mu.Lock() + db.hits[r.URL.Path]++ + db.mu.Unlock() + if db.down.Load() { + http.Error(w, "unavailable", http.StatusInternalServerError) + return + } + body, ok := db.files[r.URL.Path] + if !ok { + text, custom := db.notFound[r.URL.Path] + if !custom { + text = "not found: " + r.URL.Path + } + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, text) + return + } + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + _, _ = io.WriteString(w, body) + })) + t.Cleanup(db.server.Close) + return db +} + +func (db *fakeSumDB) requests(path string) int { + db.mu.Lock() + defer db.mu.Unlock() + return db.hits[path] +} + +func (db *fakeSumDB) totalRequests() int { + db.mu.Lock() + defer db.mu.Unlock() + total := 0 + for _, n := range db.hits { + total += n + } + return total +} + +func newSumDBTestHandler(t *testing.T, sumDBURL string, cacheMetadata bool, ttl time.Duration) *GoHandler { + t.Helper() + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = http.DefaultClient + proxy.CacheMetadata = cacheMetadata + proxy.MetadataTTL = ttl + h := NewGoHandler(proxy, "http://proxy.example") + h.SetSumDBs(map[string]string{testSumDBName: sumDBURL}) + return h +} + +func serveGoRequest(h *GoHandler, target string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil)) + return w +} + +func TestGoSumDB_SupportedProbe(t *testing.T) { + t.Run("default database", func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + h := NewGoHandler(proxy, "http://proxy.example") + if w := serveGoRequest(h, "/sumdb/sum.golang.org/supported"); w.Code != http.StatusOK { + t.Errorf("supported for default sum.golang.org: status = %d, want 200", w.Code) + } + }) + + t.Run("configured databases replace the default", func(t *testing.T) { + db := newFakeSumDB(t, nil) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + for target, want := range map[string]int{ + "/sumdb/sum.example/supported": http.StatusOK, + "/sumdb/sum.golang.org/supported": http.StatusNotFound, + "/sumdb/other.example/supported": http.StatusNotFound, + } { + if w := serveGoRequest(h, target); w.Code != want { + t.Errorf("%s: status = %d, want %d", target, w.Code, want) + } + } + if n := db.totalRequests(); n != 0 { + t.Errorf("upstream requests = %d, want 0: supported is answered by the proxy", n) + } + }) +} + +func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { + var moduleUpstreamRequests atomic.Int32 + moduleUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + moduleUpstreamRequests.Add(1) + http.NotFound(w, r) + })) + defer moduleUpstream.Close() + + const lookup = "/lookup/golang.org/x/text@v0.14.0" + db := newFakeSumDB(t, map[string]string{lookup: "lookup record"}) + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = http.DefaultClient + h := NewGoHandlerWithUpstream(proxy, "http://proxy.example", moduleUpstream.URL) + h.SetSumDBs(map[string]string{testSumDBName: db.server.URL + "/"}) + + w := serveGoRequest(h, "/sumdb/sum.example"+lookup) + if w.Code != http.StatusOK || w.Body.String() != "lookup record" { + t.Fatalf("status = %d, body = %q; want 200 with the database's response", w.Code, w.Body.String()) + } + if n := db.requests(lookup); n != 1 { + t.Errorf("database requests for %s = %d, want 1", lookup, n) + } + if n := moduleUpstreamRequests.Load(); n != 0 { + t.Errorf("module upstream requests = %d, want 0", n) + } +} + +func TestGoSumDB_LookupCachedWithinTTL(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + db := newFakeSumDB(t, map[string]string{lookup: "lookup record"}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + for i := range 2 { + w := serveGoRequest(h, "/sumdb/sum.example"+lookup) + if w.Code != http.StatusOK || w.Body.String() != "lookup record" { + t.Fatalf("request %d: status = %d, body = %q", i, w.Code, w.Body.String()) + } + } + if n := db.requests(lookup); n != 1 { + t.Errorf("database requests = %d, want 1: the second lookup is a cache hit", n) + } +} + +func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { + files := map[string]string{ + "/tile/8/0/000": "complete hash tile", + "/tile/8/data/000": "complete data tile", + "/tile/8/0/001.p/5": "partial hash tile", + "/tile/8/data/001.p/5": "partial data tile", + "/latest": "signed tree", + } + wantRequests := map[string]int{ + "/tile/8/0/000": 1, + "/tile/8/data/000": 1, + "/tile/8/0/001.p/5": 2, + "/tile/8/data/001.p/5": 2, + "/latest": 2, + } + db := newFakeSumDB(t, files) + // Every cached entry is past its TTL by the second request. + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + + for range 2 { + for path, want := range files { + w := serveGoRequest(h, "/sumdb/sum.example"+path) + if w.Code != http.StatusOK || w.Body.String() != want { + t.Fatalf("%s: status = %d, body = %q, want %q", path, w.Code, w.Body.String(), want) + } + } + } + for path, want := range wantRequests { + if n := db.requests(path); n != want { + t.Errorf("%s: database requests = %d, want %d", path, n, want) + } + } +} + +func TestGoSumDB_CompleteTilesNotStoredWithoutMetadataCache(t *testing.T) { + const tile = "/tile/8/0/000" + db := newFakeSumDB(t, map[string]string{tile: "complete hash tile"}) + h := newSumDBTestHandler(t, db.server.URL, false, time.Hour) + + for range 2 { + if w := serveGoRequest(h, "/sumdb/sum.example"+tile); w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + } + if n := db.requests(tile); n != 2 { + t.Errorf("database requests = %d, want 2 with cache_metadata off", n) + } +} + +func TestGoSumDB_OfflineFallback(t *testing.T) { + files := map[string]string{ + "/lookup/golang.org/x/text@v0.14.0": "lookup record", + "/latest": "signed tree", + "/tile/8/0/000": "complete hash tile", + "/tile/8/0/001.p/5": "partial hash tile", + } + db := newFakeSumDB(t, files) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + + for path := range files { + if w := serveGoRequest(h, "/sumdb/sum.example"+path); w.Code != http.StatusOK { + t.Fatalf("%s online: status = %d, want 200", path, w.Code) + } + } + + db.down.Store(true) + for path, want := range files { + w := serveGoRequest(h, "/sumdb/sum.example"+path) + if w.Code != http.StatusOK || w.Body.String() != want { + t.Errorf("%s offline: status = %d, body = %q, want 200 with %q", path, w.Code, w.Body.String(), want) + } + } +} + +func TestGoSumDB_NotFoundRelaysUpstreamText(t *testing.T) { + const lookup = "/lookup/github.com/google/uuid@v9.9.9" + const reason = "not found: github.com/google/uuid@v9.9.9: invalid version: unknown revision v9.9.9" + + for _, cacheMetadata := range []bool{true, false} { + t.Run(fmt.Sprintf("cache_metadata=%v", cacheMetadata), func(t *testing.T) { + db := newFakeSumDB(t, nil) + db.notFound[lookup] = reason + h := newSumDBTestHandler(t, db.server.URL, cacheMetadata, time.Hour) + + w := serveGoRequest(h, "/sumdb/sum.example"+lookup) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", w.Code) + } + if got := strings.TrimSpace(w.Body.String()); got != reason { + t.Errorf("body = %q, want the database's text %q", got, reason) + } + }) + } + + t.Run("long text is cut", func(t *testing.T) { + db := newFakeSumDB(t, nil) + db.notFound[lookup] = strings.Repeat("x", upstreamNotFoundBodyLimit+1000) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + w := serveGoRequest(h, "/sumdb/sum.example"+lookup) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", w.Code) + } + if got := w.Body.Len(); got != upstreamNotFoundBodyLimit { + t.Errorf("body length = %d, want %d", got, upstreamNotFoundBodyLimit) + } + }) +} + +func TestGoSumDB_InvalidPathsNeverReachUpstream(t *testing.T) { + db := newFakeSumDB(t, nil) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + for _, target := range []string{ + "/sumdb/sum.example", + "/sumdb/sum.example/", + "/sumdb/sum.example/other", + "/sumdb/sum.example/latest/extra", + "/sumdb/sum.example/lookup/", + "/sumdb/sum.example/lookup/golang.org/x/text", + "/sumdb/sum.example/lookup/golang.org/x/text@", + "/sumdb/sum.example/lookup/golang.org/x/text@master", + "/sumdb/sum.example/lookup/github.com/Google/uuid@v1.6.0", + "/sumdb/sum.example/lookup/../../etc/passwd@v1.0.0", + "/sumdb/sum.example/tile/x/0/000", + "/sumdb/sum.example/tile/8/0/000.p/0", + "/sumdb/sum.example/tile/8/0/000.p/256", + "/sumdb/sum.example/tile/8/0/../../../latest", + "/sumdb/unknown.example/latest", + } { + if w := serveGoRequest(h, target); w.Code != http.StatusNotFound { + t.Errorf("%s: status = %d, want 404", target, w.Code) + } + } + if n := db.totalRequests(); n != 0 { + t.Errorf("upstream requests = %d, want 0", n) + } +} + +// sumDBClientOps lets the x/mod checksum database client read through the +// proxy, keeping its configuration and cache in memory. +type sumDBClientOps struct { + t *testing.T + remote string + key string + + mu sync.Mutex + config map[string][]byte + cache map[string][]byte +} + +func newSumDBClientOps(t *testing.T, remote, key string) *sumDBClientOps { + return &sumDBClientOps{t: t, remote: remote, key: key, config: map[string][]byte{}, cache: map[string][]byte{}} +} + +func (o *sumDBClientOps) ReadRemote(path string) ([]byte, error) { + resp, err := http.Get(o.remote + path) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("GET %s: %s: %s", path, resp.Status, body) + } + return body, nil +} + +func (o *sumDBClientOps) ReadConfig(file string) ([]byte, error) { + if file == "key" { + return []byte(o.key), nil + } + o.mu.Lock() + defer o.mu.Unlock() + return o.config[file], nil +} + +func (o *sumDBClientOps) WriteConfig(file string, old, data []byte) error { + o.mu.Lock() + defer o.mu.Unlock() + if !bytes.Equal(o.config[file], old) { + return sumdb.ErrWriteConflict + } + o.config[file] = data + return nil +} + +func (o *sumDBClientOps) ReadCache(file string) ([]byte, error) { + o.mu.Lock() + defer o.mu.Unlock() + data, ok := o.cache[file] + if !ok { + return nil, errors.New("not cached") + } + return data, nil +} + +func (o *sumDBClientOps) WriteCache(file string, data []byte) { + o.mu.Lock() + defer o.mu.Unlock() + o.cache[file] = data +} + +func (o *sumDBClientOps) Log(msg string) { o.t.Log(msg) } + +func (o *sumDBClientOps) SecurityError(msg string) { + o.t.Errorf("checksum database security error: %s", msg) +} + +// TestGoSumDB_ClientRoundTrip runs the go command's checksum database client +// against a real database implementation through the proxy, so signatures, +// record proofs and tile hashes are checked end to end. A second client then +// verifies the same lookup from the proxy's cache with the database down. +func TestGoSumDB_ClientRoundTrip(t *testing.T) { + signerKey, verifierKey, err := note.GenerateKey(rand.Reader, testSumDBName) + if err != nil { + t.Fatal(err) + } + gosum := func(path, version string) ([]byte, error) { + return fmt.Appendf(nil, "%s %s h1:Y29udGVudA==\n%s %s/go.mod h1:Z28ubW9k\n", path, version, path, version), nil + } + database := sumdb.NewServer(sumdb.NewTestServer(signerKey, gosum)) + var down atomic.Bool + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if down.Load() { + http.Error(w, "unavailable", http.StatusInternalServerError) + return + } + database.ServeHTTP(w, r) + })) + defer upstream.Close() + + h := newSumDBTestHandler(t, upstream.URL, true, time.Hour) + proxy := httptest.NewServer(h.Routes()) + defer proxy.Close() + remote := proxy.URL + "/sumdb/" + testSumDBName + + client := sumdb.NewClient(newSumDBClientOps(t, remote, verifierKey)) + for _, m := range [][2]string{{"rsc.io/quote", "v1.5.2"}, {"golang.org/x/text", "v0.3.0"}, {"github.com/Google/UUID", "v1.6.0"}} { + lines, err := client.Lookup(m[0], m[1]) + if err != nil { + t.Fatalf("lookup %s@%s through the proxy: %v", m[0], m[1], err) + } + if want := m[0] + " " + m[1] + " h1:Y29udGVudA=="; len(lines) != 1 || lines[0] != want { + t.Errorf("lookup %s@%s = %q, want [%q]", m[0], m[1], lines, want) + } + } + + down.Store(true) + offline := sumdb.NewClient(newSumDBClientOps(t, remote, verifierKey)) + if _, err := offline.Lookup("rsc.io/quote", "v1.5.2"); err != nil { + t.Fatalf("lookup from the proxy cache with the database down: %v", err) + } +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 776fa8cb..26e8c2a9 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1101,6 +1101,20 @@ func JSONError(w http.ResponseWriter, status int, message string) { // ErrUpstreamNotFound indicates the upstream returned 404. var ErrUpstreamNotFound = fmt.Errorf("upstream: %w", fetch.ErrNotFound) +// upstreamNotFoundBodyLimit caps the part of an upstream 404 body kept in +// upstreamNotFoundError. +const upstreamNotFoundBodyLimit = 4 << 10 + +// upstreamNotFoundError is ErrUpstreamNotFound with the start of the +// upstream's 404 body, for handlers that relay the reason to clients. +type upstreamNotFoundError struct { + body []byte +} + +func (e *upstreamNotFoundError) Error() string { return ErrUpstreamNotFound.Error() } + +func (e *upstreamNotFoundError) Unwrap() error { return ErrUpstreamNotFound } + // ErrArtifactBlocked indicates a pre-cache security scan blocked the artifact. var ErrArtifactBlocked = errors.New("artifact blocked by security scan") @@ -1461,7 +1475,8 @@ func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, e } if resp.StatusCode == http.StatusNotFound { - return nil, ErrUpstreamNotFound + body, _ := io.ReadAll(io.LimitReader(resp.Body, upstreamNotFoundBodyLimit)) + return nil, &upstreamNotFoundError{body: body} } if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("upstream returned %d", resp.StatusCode) diff --git a/internal/server/go_sumdbs_test.go b/internal/server/go_sumdbs_test.go new file mode 100644 index 00000000..ff9dce29 --- /dev/null +++ b/internal/server/go_sumdbs_test.go @@ -0,0 +1,38 @@ +package server + +import ( + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/handler" + "github.com/go-chi/chi/v5" +) + +func TestGoSumDBsReachGoHandler(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/lookup/golang.org/x/text@v0.14.0" { + http.NotFound(w, r) + return + } + _, _ = io.WriteString(w, "lookup record") + })) + defer upstream.Close() + + cfg := config.Default() + cfg.BaseURL = "https://proxy.example" + cfg.Upstream.GoSumDBs = map[string]string{"sum.example": upstream.URL} + p := &handler.Proxy{HTTPClient: upstream.Client(), Logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + s := &Server{cfg: cfg} + r := chi.NewRouter() + s.mountProtocolHandlers(r, p) + + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/go/sumdb/sum.example/lookup/golang.org/x/text@v0.14.0", nil)) + if w.Code != http.StatusOK || w.Body.String() != "lookup record" { + t.Errorf("status = %d, body = %q; want the lookup from upstream.go_sumdbs", w.Code, w.Body.String()) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 8ee1c623..b6c93bc8 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -402,6 +402,7 @@ func (s *Server) mountProtocolHandlers(r chi.Router, proxy *handler.Proxy) { ) gemHandler := handler.NewGemHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.Gem) goHandler := handler.NewGoHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.Go) + goHandler.SetSumDBs(s.cfg.Upstream.GoSumDBs) hexHandler := handler.NewHexHandlerWithUpstreams( proxy, s.cfg.BaseURL, From 9eb88e499ffe6214f7a43cc207df966d62496212 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 09:57:43 +0200 Subject: [PATCH 02/14] Document upstream.go_sumdbs The README now mentions that checksum lookups go through GOPROXY as well, and configuration.md has a section on the new option. --- README.md | 7 +++++++ docs/configuration.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/README.md b/README.md index b85dcd36..17e6fde8 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,13 @@ export GOPROXY=http://localhost:8080/go,direct Or in your shell profile for persistence. +Checksum lookups go through the same URL: the proxy serves `sum.golang.org` at +`/go/sumdb/sum.golang.org/`, and the `go` command uses it without further +setup, so clients need no route of their own to `sum.golang.org`. The proxy +itself needs to reach `https://sum.golang.org`. Other checksum databases are +listed under `upstream.go_sumdbs`, see +[Go checksum database](docs/configuration.md#go-checksum-database). + ### Homebrew Point Homebrew's JSON API and artifact domain at the proxy: diff --git a/docs/configuration.md b/docs/configuration.md index 89e91411..0c3ceffe 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -495,6 +495,34 @@ When `upstream.arch` is empty, a single repository named `archlinux` pointing at that default. Arch Linux ARM has no default: pick a regional HTTPS mirror, not the `mirror.archlinuxarm.org` redirector. +### Go checksum database + +```yaml +upstream: + + # Named Go checksum databases, served at /go/sumdb/{name}/. + go_sumdbs: + sum.golang.org: "https://sum.golang.org" +``` + +The `go` command asks `GOPROXY` for the checksum database before contacting it +directly. The proxy answers for every name listed here and fetches lookups and +tiles from the database's URL, not from `upstream.go`. Unknown names get 404, +so the `go` command falls back to contacting that database itself. When +`upstream.go_sumdbs` is empty, `sum.golang.org` is served; configuring any +entry replaces that default. + +With `cache_metadata` enabled, lookups, partial tiles and the latest signed +tree follow `metadata_ttl` and are served from cache when the database is +unreachable. Complete tiles never change and are not revalidated once cached. +Without `cache_metadata` the requests are forwarded and nothing is stored. A +404 from the database keeps its text (up to 4 KiB), which the `go` command +prints to explain why a version is missing. + +If `upstream.go` points at a module proxy that serves the checksum database +itself, such as Athens, point the entry there instead, for example +`sum.golang.org: "https://athens.example/sumdb/sum.golang.org"`. + ### Debian archives ```yaml From e6aadb297cfd378550d73eb87d81aaa70ecf3805 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 10:18:40 +0200 Subject: [PATCH 03/14] Only claim checksum database support when the database is reachable Once a proxy answers 200 on supported, the go command sends all checksum requests there and never goes to the database directly. A proxy that cannot reach sum.golang.org would therefore break every build that needs a lookup. supported now fetches latest first, through the cache, and answers 404 if that fails and nothing is cached, so clients fall back like before. A failed check is remembered for a minute so an unreachable database doesn't make each go command wait for an upstream timeout. --- README.md | 5 ++- docs/configuration.md | 6 +++ internal/handler/go.go | 1 + internal/handler/go_sumdb.go | 64 +++++++++++++++++++++++++++- internal/handler/go_sumdb_test.go | 70 ++++++++++++++++++++++++------- 5 files changed, 128 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 17e6fde8..30e09a00 100644 --- a/README.md +++ b/README.md @@ -204,8 +204,9 @@ Or in your shell profile for persistence. Checksum lookups go through the same URL: the proxy serves `sum.golang.org` at `/go/sumdb/sum.golang.org/`, and the `go` command uses it without further -setup, so clients need no route of their own to `sum.golang.org`. The proxy -itself needs to reach `https://sum.golang.org`. Other checksum databases are +setup, so clients need no route of their own to `sum.golang.org`. If the +proxy cannot reach `https://sum.golang.org`, the `go` command contacts it +directly as before. Other checksum databases are listed under `upstream.go_sumdbs`, see [Go checksum database](docs/configuration.md#go-checksum-database). diff --git a/docs/configuration.md b/docs/configuration.md index 0c3ceffe..acee8488 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -512,6 +512,12 @@ so the `go` command falls back to contacting that database itself. When `upstream.go_sumdbs` is empty, `sum.golang.org` is served; configuring any entry replaces that default. +Once the proxy says it serves a database, the `go` command no longer contacts +that database directly. The proxy therefore only says so when it can fetch the +database's latest signed tree, or has it cached. If it cannot, it answers 404 +as for an unknown name, and clients go to the database themselves. After a +failed check the proxy waits a minute before it tries the database again. + With `cache_metadata` enabled, lookups, partial tiles and the latest signed tree follow `metadata_ttl` and are served from cache when the database is unreachable. Complete tiles never change and are not revalidated once cached. diff --git a/internal/handler/go.go b/internal/handler/go.go index 6348a1ea..e5c2f286 100644 --- a/internal/handler/go.go +++ b/internal/handler/go.go @@ -32,6 +32,7 @@ type GoHandler struct { upstreamURL string proxyURL string sumDBs map[string]string + sumDBProbes goSumDBProbes } // NewGoHandler creates a new Go module proxy handler. diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index 32665f62..ff3c7113 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -7,6 +7,8 @@ import ( "net/http" "strconv" "strings" + "sync" + "time" "github.com/git-pkgs/proxy/internal/metrics" "golang.org/x/mod/module" @@ -20,8 +22,40 @@ const ( defaultGoSumDBName = "sum.golang.org" defaultGoSumDBURL = "https://sum.golang.org" goEcosystem = "golang" + + // goSumDBProbeBackoff is how long supported keeps answering 404 after the + // proxy failed to reach a checksum database, before it tries again. + goSumDBProbeBackoff = time.Minute ) +// goSumDBProbes remembers when the proxy last failed to reach each checksum +// database, so a database that is unreachable does not make every go command +// wait for an upstream timeout before it falls back. +type goSumDBProbes struct { + mu sync.Mutex + failed map[string]time.Time +} + +func (p *goSumDBProbes) recentlyFailed(name string) bool { + p.mu.Lock() + defer p.mu.Unlock() + failedAt, ok := p.failed[name] + return ok && time.Since(failedAt) < goSumDBProbeBackoff +} + +func (p *goSumDBProbes) record(name string, reachable bool) { + p.mu.Lock() + defer p.mu.Unlock() + if reachable { + delete(p.failed, name) + return + } + if p.failed == nil { + p.failed = make(map[string]time.Time) + } + p.failed[name] = time.Now() +} + // SetSumDBs sets the checksum databases served at /sumdb/{name}/, mapping // each database name to its base URL. An empty map keeps the default, // sum.golang.org. @@ -49,7 +83,11 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str return } if path == "supported" { - w.WriteHeader(http.StatusOK) + if h.sumDBReachable(r, name, sumDBURL) { + w.WriteHeader(http.StatusOK) + } else { + http.NotFound(w, r) + } return } immutable, valid := parseGoSumDBPath(path) @@ -65,6 +103,30 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str h.proxySumDB(w, r, sumDBURL+"/"+path, cacheKey) } +// sumDBReachable reports whether the proxy can serve a checksum database: it +// can fetch the latest signed tree, or has it cached. Once supported answers +// 200 the go command uses only the proxy for that database and never falls +// back to contacting it directly, so a database the proxy cannot reach gets +// 404 instead. The probe goes through the same cache entry as a latest +// request, so it costs at most one upstream request per metadata TTL. +func (h *GoHandler) sumDBReachable(r *http.Request, name, sumDBURL string) bool { + if h.sumDBProbes.recentlyFailed(name) { + return false + } + cacheKey := goSumDBCacheKey(name, sumDBURL, "latest") + _, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), goEcosystem, cacheKey, sumDBURL+"/latest", "identity", nil, "*/*") + if err != nil && r.Context().Err() != nil { + // The client went away; that says nothing about the database. + return false + } + h.sumDBProbes.record(name, err == nil) + if err != nil { + h.proxy.Logger.Info("checksum database unreachable, sending go clients to it directly", + "name", name, "url", sumDBURL, "error", err) + } + return err == nil +} + // parseGoSumDBPath reports whether path is a request the checksum database // protocol defines and whether its response can never change. Only complete // tiles are immutable; partial tiles, latest and lookups follow the metadata diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index a9ff1dfa..e74c5953 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -94,19 +94,22 @@ func serveGoRequest(h *GoHandler, target string) *httptest.ResponseRecorder { return w } -func TestGoSumDB_SupportedProbe(t *testing.T) { - t.Run("default database", func(t *testing.T) { - proxy, _, _, _ := setupTestProxy(t) - h := NewGoHandler(proxy, "http://proxy.example") - if w := serveGoRequest(h, "/sumdb/sum.golang.org/supported"); w.Code != http.StatusOK { - t.Errorf("supported for default sum.golang.org: status = %d, want 200", w.Code) - } - }) +func TestGoSumDB_DefaultsToSumGolangOrg(t *testing.T) { + h := NewGoHandler(nil, "http://proxy.example") + if got := h.sumDBs[defaultGoSumDBName]; got != defaultGoSumDBURL { + t.Errorf("default checksum database = %q, want %q", got, defaultGoSumDBURL) + } + h.SetSumDBs(nil) + if got := h.sumDBs[defaultGoSumDBName]; got != defaultGoSumDBURL || len(h.sumDBs) != 1 { + t.Errorf("SetSumDBs(nil) = %v, want only the default", h.sumDBs) + } +} - t.Run("configured databases replace the default", func(t *testing.T) { - db := newFakeSumDB(t, nil) - h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) +func TestGoSumDB_SupportedProbe(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + for range 2 { for target, want := range map[string]int{ "/sumdb/sum.example/supported": http.StatusOK, "/sumdb/sum.golang.org/supported": http.StatusNotFound, @@ -116,10 +119,47 @@ func TestGoSumDB_SupportedProbe(t *testing.T) { t.Errorf("%s: status = %d, want %d", target, w.Code, want) } } - if n := db.totalRequests(); n != 0 { - t.Errorf("upstream requests = %d, want 0: supported is answered by the proxy", n) - } - }) + } + if n := db.totalRequests(); n != 1 { + t.Errorf("upstream requests = %d, want 1: the probe fetches latest once, then uses the cache", n) + } +} + +// TestGoSumDB_SupportedFallsBackWhenUnreachable covers the go command's only +// way back to the database: once supported answers 200 it never contacts the +// database directly, so a database the proxy cannot reach must get 404. +func TestGoSumDB_SupportedFallsBackWhenUnreachable(t *testing.T) { + for _, cacheMetadata := range []bool{true, false} { + t.Run(fmt.Sprintf("cache_metadata=%v", cacheMetadata), func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + db.down.Store(true) + h := newSumDBTestHandler(t, db.server.URL, cacheMetadata, time.Hour) + + for i := range 2 { + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { + t.Fatalf("request %d: status = %d, want 404", i, w.Code) + } + } + if n := db.requests("/latest"); n != 1 { + t.Errorf("probes = %d, want 1: a failed probe is not repeated within %s", n, goSumDBProbeBackoff) + } + }) + } +} + +// TestGoSumDB_SupportedWithCachedTreeWhileUnreachable keeps the offline case: +// a proxy that has served the database before can still serve it from cache. +func TestGoSumDB_SupportedWithCachedTreeWhileUnreachable(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) + } + db.down.Store(true) + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Errorf("supported with a cached tree: status = %d, want 200", w.Code) + } } func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { From 9a93ce05b4af6c35c7d3fdc27cc974387683746f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 10:19:39 +0200 Subject: [PATCH 04/14] Check tile sizes before caching them Complete tiles are never fetched again once cached, so a bad response would stick around until someone deletes it by hand. That could be an error page from a proxy in between that comes back with 200. A hash tile is always W*32 bytes, so anything else is refused now and not stored, and the same check runs when a cached tile is served. Data tiles have no fixed size and only must not be empty. A cached tile that fails the check is fetched again without asking the database whether it changed. Past the TTL the cache would ask, and a database that sends an ETag would answer 304 and hand back the damaged copy every time. The fake database in the tests now guards its maps with the mutex, since the tests change them between requests. --- docs/configuration.md | 3 +- internal/handler/go_sumdb.go | 74 +++++++++++++++++++++++------ internal/handler/go_sumdb_test.go | 77 +++++++++++++++++++++++++------ 3 files changed, 126 insertions(+), 28 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index acee8488..38f79a41 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -520,7 +520,8 @@ failed check the proxy waits a minute before it tries the database again. With `cache_metadata` enabled, lookups, partial tiles and the latest signed tree follow `metadata_ttl` and are served from cache when the database is -unreachable. Complete tiles never change and are not revalidated once cached. +unreachable. Complete tiles never change and are not revalidated once cached; +a tile of the wrong size is refused and not stored. Without `cache_metadata` the requests are forwarded and nothing is stored. A 404 from the database keeps its text (up to 4 KiB), which the `go` command prints to explain why a version is missing. diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index ff3c7113..ac7f4043 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -1,9 +1,11 @@ package handler import ( + "context" "crypto/sha256" "encoding/hex" "errors" + "fmt" "net/http" "strconv" "strings" @@ -97,10 +99,15 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str } cacheKey := goSumDBCacheKey(name, sumDBURL, path) - if immutable && h.serveCachedSumDBTile(w, r, cacheKey) { + check := goSumDBTileCheck(path) + served, damaged := false, false + if immutable { + served, damaged = h.serveCachedSumDBTile(w, r, cacheKey, check) + } + if served { return } - h.proxySumDB(w, r, sumDBURL+"/"+path, cacheKey) + h.proxySumDB(w, r, sumDBURL+"/"+path, cacheKey, check, damaged) } // sumDBReachable reports whether the proxy can serve a checksum database: it @@ -145,6 +152,28 @@ func parseGoSumDBPath(path string) (immutable, valid bool) { return tile.W == 1<= 0 && len(body) != tile.W*tlog.HashSize { + return fmt.Errorf("checksum database tile %s has %d bytes, want %d", path, len(body), tile.W*tlog.HashSize) + } + if len(body) == 0 { + return fmt.Errorf("checksum database tile %s is empty", path) + } + return nil + } +} + // validGoSumDBLookup checks an escaped module@version the way the checksum // database does. Requiring a semantic version also keeps characters such as // '#' and '%' out of the upstream URL. @@ -169,20 +198,24 @@ func goSumDBCacheKey(name, sumDBURL, path string) string { } // serveCachedSumDBTile serves a complete tile from the metadata cache -// regardless of its age, since its content never changes. It reports false -// when the tile is not cached. -func (h *GoHandler) serveCachedSumDBTile(w http.ResponseWriter, r *http.Request, cacheKey string) bool { +// regardless of its age, since its content never changes. It reports +// served false when the tile is not cached, and damaged as well when the +// cached copy fails check. +func (h *GoHandler) serveCachedSumDBTile(w http.ResponseWriter, r *http.Request, cacheKey string, check func([]byte) error) (served, damaged bool) { p := h.proxy if !p.CacheMetadata || p.DB == nil { - return false + return false, false } entry, err := p.DB.GetMetadataCache(goEcosystem, cacheKey) if err != nil || entry == nil { - return false + return false, false } body, contentType, err := p.readCachedMetadata(r.Context(), entry, nil) if err != nil { - return false + return false, false + } + if check != nil && check(body) != nil { + return false, true } metrics.RecordCacheHit(goEcosystem) @@ -190,7 +223,7 @@ func (h *GoHandler) serveCachedSumDBTile(w http.ResponseWriter, r *http.Request, w.Header().Set(headerETag, entry.ETag.String) if ifNoneMatchHits(r.Header.Get("If-None-Match"), entry.ETag.String) { w.WriteHeader(http.StatusNotModified) - return true + return true, false } } w.Header().Set(headerContentType, contentType) @@ -200,20 +233,22 @@ func (h *GoHandler) serveCachedSumDBTile(w http.ResponseWriter, r *http.Request, } w.WriteHeader(http.StatusOK) _, _ = w.Write(body) - return true + return true, false } // proxySumDB fetches a checksum database response through the metadata // cache. Unlike ProxyCached it relays the start of an upstream 404 body: the -// go command prints it, and it says why a module version is missing. -func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, upstreamURL, cacheKey string) { +// go command prints it, and it says why a module version is missing. check, +// when set, must accept a response before it is cached or served. damaged +// says the cached copy failed check. +func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) { p := h.proxy if !p.CacheMetadata { p.proxyMetadataStream(w, r, upstreamURL, "identity", "*/*") return } - body, contentType, contentEncoding, err := p.fetchOrCacheMetadata(r.Context(), goEcosystem, cacheKey, upstreamURL, "identity", nil, "*/*") + body, contentType, contentEncoding, err := h.fetchSumDB(r.Context(), upstreamURL, cacheKey, check, damaged) if err != nil { var notFound *upstreamNotFoundError switch { @@ -233,3 +268,16 @@ func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, upstreamU p.writeMetadataCachedResponseWithEncoding(w, r, goEcosystem, cacheKey, body, contentType, contentEncoding) } + +// fetchSumDB fetches a checksum database response through the metadata +// cache. A cached copy that failed check is fetched again without +// revalidating it: past the TTL, a database that sends an ETag would answer +// 304 and hand back the damaged copy for good. +func (h *GoHandler) fetchSumDB(ctx context.Context, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) ([]byte, string, string, error) { + p := h.proxy + if damaged { + res := p.fetchMetadataFromUpstream(ctx, goEcosystem, cacheKey, upstreamURL, "*/*", "identity", check, nil) + return res.body, res.contentType, res.contentEncoding, res.err + } + return p.fetchOrCacheMetadata(ctx, goEcosystem, cacheKey, upstreamURL, "identity", check, "*/*") +} diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index e74c5953..744a45f3 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -16,36 +16,45 @@ import ( "golang.org/x/mod/sumdb" "golang.org/x/mod/sumdb/note" + "golang.org/x/mod/sumdb/tlog" ) const testSumDBName = "sum.example" +// hashTile returns a hash tile body of the given width. +func hashTile(width int, fill string) string { + return strings.Repeat(fill, width*tlog.HashSize) +} + // fakeSumDB is a checksum database upstream serving fixed files. Unknown // paths get 404 with the body from notFound, or a default text. type fakeSumDB struct { - server *httptest.Server + server *httptest.Server + + mu sync.Mutex files map[string]string notFound map[string]string - - mu sync.Mutex - hits map[string]int - down atomic.Bool + hits map[string]int + down atomic.Bool } func newFakeSumDB(t *testing.T, files map[string]string) *fakeSumDB { t.Helper() + if files == nil { + files = map[string]string{} + } db := &fakeSumDB{files: files, notFound: map[string]string{}, hits: map[string]int{}} db.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { db.mu.Lock() db.hits[r.URL.Path]++ + body, ok := db.files[r.URL.Path] + text, custom := db.notFound[r.URL.Path] db.mu.Unlock() if db.down.Load() { http.Error(w, "unavailable", http.StatusInternalServerError) return } - body, ok := db.files[r.URL.Path] if !ok { - text, custom := db.notFound[r.URL.Path] if !custom { text = "not found: " + r.URL.Path } @@ -61,6 +70,18 @@ func newFakeSumDB(t *testing.T, files map[string]string) *fakeSumDB { return db } +func (db *fakeSumDB) setFile(path, body string) { + db.mu.Lock() + defer db.mu.Unlock() + db.files[path] = body +} + +func (db *fakeSumDB) setNotFound(path, text string) { + db.mu.Lock() + defer db.mu.Unlock() + db.notFound[path] = text +} + func (db *fakeSumDB) requests(path string) int { db.mu.Lock() defer db.mu.Unlock() @@ -208,9 +229,9 @@ func TestGoSumDB_LookupCachedWithinTTL(t *testing.T) { func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { files := map[string]string{ - "/tile/8/0/000": "complete hash tile", + "/tile/8/0/000": hashTile(256, "a"), "/tile/8/data/000": "complete data tile", - "/tile/8/0/001.p/5": "partial hash tile", + "/tile/8/0/001.p/5": hashTile(5, "b"), "/tile/8/data/001.p/5": "partial data tile", "/latest": "signed tree", } @@ -240,9 +261,37 @@ func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { } } +// TestGoSumDB_BadTilesAreNotCached guards complete tiles, which are never +// revalidated once cached: a response of the wrong size must not be stored. +func TestGoSumDB_BadTilesAreNotCached(t *testing.T) { + const tile = "/tile/8/0/000" + db := newFakeSumDB(t, map[string]string{ + tile: "please log in", + "/tile/8/data/000.p/3": "", + }) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + if w := serveGoRequest(h, "/sumdb/sum.example"+tile); w.Code != http.StatusBadGateway { + t.Fatalf("wrong-size tile: status = %d, want 502", w.Code) + } + if w := serveGoRequest(h, "/sumdb/sum.example/tile/8/data/000.p/3"); w.Code != http.StatusBadGateway { + t.Errorf("empty data tile: status = %d, want 502", w.Code) + } + + want := hashTile(256, "a") + db.setFile(tile, want) + w := serveGoRequest(h, "/sumdb/sum.example"+tile) + if w.Code != http.StatusOK || w.Body.String() != want { + t.Fatalf("after upstream recovered: status = %d, body length %d, want the %d-byte tile", w.Code, w.Body.Len(), len(want)) + } + if n := db.requests(tile); n != 2 { + t.Errorf("upstream requests = %d, want 2: the bad response was not cached", n) + } +} + func TestGoSumDB_CompleteTilesNotStoredWithoutMetadataCache(t *testing.T) { const tile = "/tile/8/0/000" - db := newFakeSumDB(t, map[string]string{tile: "complete hash tile"}) + db := newFakeSumDB(t, map[string]string{tile: hashTile(256, "a")}) h := newSumDBTestHandler(t, db.server.URL, false, time.Hour) for range 2 { @@ -259,8 +308,8 @@ func TestGoSumDB_OfflineFallback(t *testing.T) { files := map[string]string{ "/lookup/golang.org/x/text@v0.14.0": "lookup record", "/latest": "signed tree", - "/tile/8/0/000": "complete hash tile", - "/tile/8/0/001.p/5": "partial hash tile", + "/tile/8/0/000": hashTile(256, "a"), + "/tile/8/0/001.p/5": hashTile(5, "b"), } db := newFakeSumDB(t, files) h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) @@ -287,7 +336,7 @@ func TestGoSumDB_NotFoundRelaysUpstreamText(t *testing.T) { for _, cacheMetadata := range []bool{true, false} { t.Run(fmt.Sprintf("cache_metadata=%v", cacheMetadata), func(t *testing.T) { db := newFakeSumDB(t, nil) - db.notFound[lookup] = reason + db.setNotFound(lookup, reason) h := newSumDBTestHandler(t, db.server.URL, cacheMetadata, time.Hour) w := serveGoRequest(h, "/sumdb/sum.example"+lookup) @@ -302,7 +351,7 @@ func TestGoSumDB_NotFoundRelaysUpstreamText(t *testing.T) { t.Run("long text is cut", func(t *testing.T) { db := newFakeSumDB(t, nil) - db.notFound[lookup] = strings.Repeat("x", upstreamNotFoundBodyLimit+1000) + db.setNotFound(lookup, strings.Repeat("x", upstreamNotFoundBodyLimit+1000)) h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) w := serveGoRequest(h, "/sumdb/sum.example"+lookup) From f47dd2f91c5d198b2db8c470448e08efdc9c4b4b Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 10:19:46 +0200 Subject: [PATCH 05/14] Say that the 4 KiB limit on 404 text only applies with the metadata cache --- docs/configuration.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 38f79a41..02d39b80 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -522,9 +522,11 @@ With `cache_metadata` enabled, lookups, partial tiles and the latest signed tree follow `metadata_ttl` and are served from cache when the database is unreachable. Complete tiles never change and are not revalidated once cached; a tile of the wrong size is refused and not stored. -Without `cache_metadata` the requests are forwarded and nothing is stored. A -404 from the database keeps its text (up to 4 KiB), which the `go` command -prints to explain why a version is missing. +Without `cache_metadata` the requests are forwarded and nothing is stored. + +A 404 from the database keeps its text, which the `go` command prints to +explain why a version is missing. With `cache_metadata` enabled the text is +cut at 4 KiB; without it the response is passed on unchanged. If `upstream.go` points at a module proxy that serves the checksum database itself, such as Athens, point the entry there instead, for example From c1a597d2d874c16a32fdb805ccb1106e47251d4f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 11:31:01 +0200 Subject: [PATCH 06/14] Don't let the supported check hang, and don't take a block page for an answer Two problems with the check behind supported. The fetch it waits on is shared with other callers and only bounded by the upstream timeout, which defaults to 30 seconds. A firewall that silently drops packets to sum.golang.org would stall the first go command every minute for that long, and the go command itself has no timeout on this request. Now we wait five seconds at most; the fetch carries on in the background, records how it went and fills the cache for the next check. A timeout only counts as a failure if the fetch hasn't succeeded in the meantime. The fetch reports no error when it falls back to the cached tree after the database failed, so the metadata result now says when it did, and the check treats that as the database not answering. A fresh cached tree (within metadata_ttl) still lets a timed-out check say yes, since the fetch only falls back once upstream has failed, which a silent database delays past our timeout. An older tree does not count: once the proxy has lost the database, clients go there directly, as they would without the proxy. And the check believed any 200. Egress proxies tend to answer blocked hosts with a 200 HTML page, which we would have cached as the signed tree and taken as proof that the database is reachable. latest now has to start with "go.sum database tree", or it is refused like a bad tile. Lookups had the same gap. A block page cached as a lookup would also replace the good copy that is served during an outage, so a lookup now has to be a record followed by a signed tree, the way the go command reads it. --- docs/configuration.md | 8 +- internal/handler/go_sumdb.go | 175 +++++++++++++++++++++++++++++------ internal/handler/handler.go | 5 +- 3 files changed, 156 insertions(+), 32 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 02d39b80..be7df9e6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -514,9 +514,11 @@ entry replaces that default. Once the proxy says it serves a database, the `go` command no longer contacts that database directly. The proxy therefore only says so when it can fetch the -database's latest signed tree, or has it cached. If it cannot, it answers 404 -as for an unknown name, and clients go to the database themselves. After a -failed check the proxy waits a minute before it tries the database again. +database's latest signed tree, or has a fresh one cached (fetched within +`metadata_ttl`). If it cannot, it answers 404 as for an unknown name, and +clients go to the database themselves, as they would without the proxy. The +check waits five seconds at most, and after a failed check the proxy waits a +minute before it tries the database again. With `cache_metadata` enabled, lookups, partial tiles and the latest signed tree follow `metadata_ttl` and are served from cache when the database is diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index ac7f4043..0e0bcbf6 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/metrics" "golang.org/x/mod/module" "golang.org/x/mod/semver" @@ -28,14 +29,29 @@ const ( // goSumDBProbeBackoff is how long supported keeps answering 404 after the // proxy failed to reach a checksum database, before it tries again. goSumDBProbeBackoff = time.Minute + // goSumDBProbeTimeout is how long supported waits for the database. The + // go command's own request has no timeout, so a database that drops + // packets must not hold every build for the full upstream timeout. + goSumDBProbeTimeout = 5 * time.Second ) // goSumDBProbes remembers when the proxy last failed to reach each checksum // database, so a database that is unreachable does not make every go command // wait for an upstream timeout before it falls back. type goSumDBProbes struct { - mu sync.Mutex - failed map[string]time.Time + // timeout overrides goSumDBProbeTimeout when set (tests). + timeout time.Duration + + mu sync.Mutex + failed map[string]time.Time + succeeded map[string]time.Time +} + +func (p *goSumDBProbes) probeTimeout() time.Duration { + if p.timeout > 0 { + return p.timeout + } + return goSumDBProbeTimeout } func (p *goSumDBProbes) recentlyFailed(name string) bool { @@ -50,6 +66,24 @@ func (p *goSumDBProbes) record(name string, reachable bool) { defer p.mu.Unlock() if reachable { delete(p.failed, name) + if p.succeeded == nil { + p.succeeded = make(map[string]time.Time) + } + p.succeeded[name] = time.Now() + return + } + if p.failed == nil { + p.failed = make(map[string]time.Time) + } + p.failed[name] = time.Now() +} + +// recordTimeout records a failure for a probe started at start, unless the +// fetch it gave up waiting for has succeeded in the meantime. +func (p *goSumDBProbes) recordTimeout(name string, start time.Time) { + p.mu.Lock() + defer p.mu.Unlock() + if !p.succeeded[name].Before(start) { return } if p.failed == nil { @@ -99,7 +133,7 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str } cacheKey := goSumDBCacheKey(name, sumDBURL, path) - check := goSumDBTileCheck(path) + check := goSumDBResponseCheck(path) served, damaged := false, false if immutable { served, damaged = h.serveCachedSumDBTile(w, r, cacheKey, check) @@ -115,23 +149,91 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str // 200 the go command uses only the proxy for that database and never falls // back to contacting it directly, so a database the proxy cannot reach gets // 404 instead. The probe goes through the same cache entry as a latest -// request, so it costs at most one upstream request per metadata TTL. +// request: with cache_metadata it costs at most one upstream request per +// metadata TTL, without it each probe fetches latest. func (h *GoHandler) sumDBReachable(r *http.Request, name, sumDBURL string) bool { + cacheKey := goSumDBCacheKey(name, sumDBURL, "latest") + treeCheck := goSumDBResponseCheck("latest") if h.sumDBProbes.recentlyFailed(name) { - return false + return h.cachedTreeServes(r.Context(), cacheKey, treeCheck) } - cacheKey := goSumDBCacheKey(name, sumDBURL, "latest") - _, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), goEcosystem, cacheKey, sumDBURL+"/latest", "identity", nil, "*/*") - if err != nil && r.Context().Err() != nil { + // The fetch is shared with concurrent callers and runs detached from this + // request, so it is bounded by the upstream timeout, not by ours. Wait + // for it only up to the probe timeout; it finishes in the background, + // records its own outcome and fills the cache for the next probe. + start := time.Now() + done := make(chan error, 1) + go func() { + done <- h.probeSumDB(context.WithoutCancel(r.Context()), name, cacheKey, sumDBURL, treeCheck) + }() + var err error + select { + case err = <-done: + case <-time.After(h.sumDBProbes.probeTimeout()): + err = fmt.Errorf("no answer within %s", h.sumDBProbes.probeTimeout()) + h.sumDBProbes.recordTimeout(name, start) + case <-r.Context().Done(): // The client went away; that says nothing about the database. return false } + if err == nil { + return true + } + // The stale-cache fallback inside the fetch only runs once upstream has + // failed, which a silent database delays past the probe timeout. A tree + // cached earlier may still serve lookups, so look for it here. + if h.cachedTreeServes(r.Context(), cacheKey, treeCheck) { + return true + } + h.proxy.Logger.Info("checksum database unreachable, sending go clients to it directly", + "name", name, "url", sumDBURL, "error", err) + return false +} + +// probeSumDB fetches the database's latest tree and records whether the +// database answered. A tree served from the cache within its TTL counts as +// an answer; one served because upstream failed does not. +func (h *GoHandler) probeSumDB(ctx context.Context, name, cacheKey, sumDBURL string, check func([]byte) error) error { + res := h.proxy.lookupMetadata(ctx, goEcosystem, cacheKey, sumDBURL+"/latest", "*/*", "identity", check) + err := res.err + if err == nil && res.stale { + err = errors.New("checksum database did not answer, tree served from cache") + } h.sumDBProbes.record(name, err == nil) + return err +} + +// cachedTreeServes reports whether a cached tree lets the proxy answer for +// the database although the database did not: only while the tree is fresh, +// so clients fall back to the database once the proxy has lost it. +func (h *GoHandler) cachedTreeServes(ctx context.Context, cacheKey string, check func([]byte) error) bool { + entry, _, _, ok := h.readCachedSumDB(ctx, cacheKey, check) + return ok && h.treeFresh(entry) +} + +// treeFresh reports whether a cached tree is within the metadata TTL. +func (h *GoHandler) treeFresh(entry *database.MetadataCacheEntry) bool { + ttl := h.proxy.MetadataTTL + return ttl > 0 && entry.FetchedAt.Valid && time.Since(entry.FetchedAt.Time) < ttl +} + +// readCachedSumDB reads a checksum database response from the metadata +// cache regardless of its age. It reports false when nothing usable is +// cached. +func (h *GoHandler) readCachedSumDB(ctx context.Context, cacheKey string, check func([]byte) error) (entry *database.MetadataCacheEntry, body []byte, contentType string, ok bool) { + p := h.proxy + if !p.CacheMetadata || p.DB == nil { + return nil, nil, "", false + } + entry, err := p.DB.GetMetadataCache(goEcosystem, cacheKey) + if err != nil || entry == nil { + return nil, nil, "", false + } + body, contentType, err = p.readCachedMetadata(ctx, entry, check) if err != nil { - h.proxy.Logger.Info("checksum database unreachable, sending go clients to it directly", - "name", name, "url", sumDBURL, "error", err) + return nil, nil, "", false } - return err == nil + return entry, body, contentType, true } // parseGoSumDBPath reports whether path is a request the checksum database @@ -152,13 +254,38 @@ func parseGoSumDBPath(path string) (immutable, valid bool) { return tile.W == 1< Date: Sat, 10 Oct 2026 11:31:01 +0200 Subject: [PATCH 07/14] Test the probe timeout, the block pages and a damaged cached tile Also the cases around a silent or slow database: a fresh cached tree still gets a yes on supported, a stale one does not, a late answer from the database clears the remembered failure, a fallback to the cached tree is not mistaken for an answer, and an answer still counts when the cache write fails afterwards. The damaged tile test guards the size check on the cached copy of a complete tile: it shortens the stored blob and expects the tile to be fetched again, also past the TTL from a database that sends an ETag. A block page in place of a lookup must not replace the cached record, not even one that starts like a record. The lookup fixtures now look like real records, since lookups are checked too. --- internal/handler/go_sumdb_test.go | 309 ++++++++++++++++++++++++++++-- 1 file changed, 296 insertions(+), 13 deletions(-) diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index 744a45f3..a849ad83 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -19,7 +19,13 @@ import ( "golang.org/x/mod/sumdb/tlog" ) -const testSumDBName = "sum.example" +const ( + testSumDBName = "sum.example" + // testSumDBTree looks like a signed tree as far as the proxy checks. + testSumDBTree = "go.sum database tree\n1\nhash\n\n— sum.example signature\n" + // testSumDBRecord looks like a lookup answer as far as the proxy checks. + testSumDBRecord = "1\ngolang.org/x/text v0.14.0 h1:hash=\ngolang.org/x/text v0.14.0/go.mod h1:hash=\n\n" + testSumDBTree +) // hashTile returns a hash tile body of the given width. func hashTile(width int, fill string) string { @@ -127,7 +133,7 @@ func TestGoSumDB_DefaultsToSumGolangOrg(t *testing.T) { } func TestGoSumDB_SupportedProbe(t *testing.T) { - db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) for range 2 { @@ -144,6 +150,9 @@ func TestGoSumDB_SupportedProbe(t *testing.T) { if n := db.totalRequests(); n != 1 { t.Errorf("upstream requests = %d, want 1: the probe fetches latest once, then uses the cache", n) } + if h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("a probe served from the fresh cache was recorded as a failure") + } } // TestGoSumDB_SupportedFallsBackWhenUnreachable covers the go command's only @@ -152,7 +161,7 @@ func TestGoSumDB_SupportedProbe(t *testing.T) { func TestGoSumDB_SupportedFallsBackWhenUnreachable(t *testing.T) { for _, cacheMetadata := range []bool{true, false} { t.Run(fmt.Sprintf("cache_metadata=%v", cacheMetadata), func(t *testing.T) { - db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) db.down.Store(true) h := newSumDBTestHandler(t, db.server.URL, cacheMetadata, time.Hour) @@ -171,15 +180,242 @@ func TestGoSumDB_SupportedFallsBackWhenUnreachable(t *testing.T) { // TestGoSumDB_SupportedWithCachedTreeWhileUnreachable keeps the offline case: // a proxy that has served the database before can still serve it from cache. func TestGoSumDB_SupportedWithCachedTreeWhileUnreachable(t *testing.T) { - db := newFakeSumDB(t, map[string]string{"/latest": "signed tree"}) + t.Run("fresh tree", func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) + } + db.down.Store(true) + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Errorf("supported with a fresh cached tree: status = %d, want 200", w.Code) + } + }) + + // A stale tree does not count: clients that can reach the database + // themselves should go there. + t.Run("stale tree", func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) + } + db.down.Store(true) + for i := range 2 { // second probe hits the remembered failure + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { + t.Errorf("probe %d: status = %d, want 404", i, w.Code) + } + } + if n := db.requests("/latest"); n != 2 { + t.Errorf("upstream requests = %d, want 2: the failure is remembered", n) + } + }) +} + +// TestGoSumDB_SupportedGivesUpOnSilentDatabase: a database that accepts the +// connection but never answers (a firewall dropping packets) must not hold +// the go command, which waits for supported without a timeout of its own. +func TestGoSumDB_SupportedGivesUpOnSilentDatabase(t *testing.T) { + release := make(chan struct{}) + db := newFakeSumDB(t, nil) + t.Cleanup(func() { close(release) }) // runs before the server closes + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + }) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + h.sumDBProbes.timeout = 50 * time.Millisecond + + answered := make(chan *httptest.ResponseRecorder, 1) + go func() { answered <- serveGoRequest(h, "/sumdb/sum.example/supported") }() + select { + case w := <-answered: + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", w.Code) + } + case <-time.After(time.Second): + t.Fatal("supported still waiting for the database after a second") + } +} + +// TestGoSumDB_SupportedWithStaleTreeWhileSilent: a silent database holds +// the fetch past the probe timeout. The stale tree does not count, and the +// second probe must answer at once from the remembered failure. +func TestGoSumDB_SupportedWithStaleTreeWhileSilent(t *testing.T) { + release := make(chan struct{}) + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + t.Cleanup(func() { close(release) }) h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBProbes.timeout = 50 * time.Millisecond if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { t.Fatalf("latest: status = %d, want 200", w.Code) } - db.down.Store(true) + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + }) + + for i := range 2 { // second probe hits the remembered failure + answered := make(chan *httptest.ResponseRecorder, 1) + go func() { answered <- serveGoRequest(h, "/sumdb/sum.example/supported") }() + select { + case w := <-answered: + if w.Code != http.StatusNotFound { + t.Fatalf("probe %d: status = %d, want 404", i, w.Code) + } + case <-time.After(time.Second): + t.Fatalf("probe %d: supported still waiting after a second", i) + } + } +} + +// TestGoSumDB_SlowProbeSuccessClearsFailure: a database that answers after +// the probe timeout gets a 404 the first time, but once its answer is in +// the cache, supported says yes without waiting for the next backoff. +func TestGoSumDB_SlowProbeSuccessClearsFailure(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + inner := db.server.Config.Handler + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(100 * time.Millisecond) + inner.ServeHTTP(w, r) + }) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + h.sumDBProbes.timeout = 10 * time.Millisecond + + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { + t.Fatalf("first probe: status = %d, want 404", w.Code) + } + deadline := time.Now().Add(5 * time.Second) + for h.sumDBProbes.recentlyFailed(testSumDBName) { + if time.Now().After(deadline) { + t.Fatal("the late fetch never cleared the failure") + } + time.Sleep(10 * time.Millisecond) + } if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { - t.Errorf("supported with a cached tree: status = %d, want 200", w.Code) + t.Errorf("after the late answer: status = %d, want 200", w.Code) + } + if n := db.requests("/latest"); n != 1 { + t.Errorf("upstream requests = %d, want 1: the second probe is a cache hit", n) + } +} + +// TestGoSumDB_StaleFallbackIsNotAnAnswer: the fetch reports no error when +// it falls back to the cached tree after the database failed. That must +// not count as the database answering, or the failure would be forgotten +// and every probe would wait for the timeout again. +func TestGoSumDB_StaleFallbackIsNotAnAnswer(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBProbes.timeout = 10 * time.Millisecond + + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) + } + inner := db.server.Config.Handler + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(100 * time.Millisecond) + inner.ServeHTTP(w, r) + }) + db.down.Store(true) + + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", w.Code) + } + time.Sleep(300 * time.Millisecond) // the fetch has failed and fallen back by now + if !h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("the stale fallback cleared the remembered failure") + } +} + +// TestGoSumDB_AnswerCountsWhenCacheWriteFails: an answer from the database +// is an answer even when it could not be cached afterwards, whether or not +// a stale tree was cached before. +func TestGoSumDB_AnswerCountsWhenCacheWriteFails(t *testing.T) { + for _, staleTree := range []bool{false, true} { + t.Run(fmt.Sprintf("stale tree cached=%v", staleTree), func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + if staleTree { + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) + } + } + h.proxy.Storage.(*mockStorage).storeErr = errors.New("disk full") + + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Errorf("status = %d, want 200: the database answered", w.Code) + } + if h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("an answered probe was recorded as a failure") + } + }) + } +} + +func TestGoSumDBProbes_TimeoutDoesNotOverwriteLaterSuccess(t *testing.T) { + var probes goSumDBProbes + start := time.Now() + probes.record(testSumDBName, true) + probes.recordTimeout(testSumDBName, start) + if probes.recentlyFailed(testSumDBName) { + t.Error("a success recorded after the probe started was overwritten by its timeout") + } + probes.recordTimeout(testSumDBName, time.Now().Add(time.Second)) + if !probes.recentlyFailed(testSumDBName) { + t.Error("a timeout with no later success was not recorded") + } +} + +// TestGoSumDB_SupportedRefusesBlockPage: egress proxies answer blocked hosts +// with a 200 page. That is not a reachable database, and must not be cached +// as its signed tree either. +func TestGoSumDB_SupportedRefusesBlockPage(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": "access to this site is blocked"}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { + t.Errorf("supported: status = %d, want 404", w.Code) + } + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusBadGateway { + t.Errorf("latest: status = %d, want 502 for a block page", w.Code) + } + + // The page was not cached: once the database answers, its tree is served. + db.setFile("/latest", testSumDBTree) + w := serveGoRequest(h, "/sumdb/sum.example/latest") + if w.Code != http.StatusOK || w.Body.String() != testSumDBTree { + t.Errorf("latest after recovery: status = %d, body = %q, want the signed tree", w.Code, w.Body.String()) + } +} + +// TestGoSumDB_LookupRefusesBlockPage: a block page in place of a lookup must +// not be cached, where it would replace the copy served during an outage. +// That includes a page that happens to start like a record. +func TestGoSumDB_LookupRefusesBlockPage(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + pages := map[string]string{ + "html": "access to this site is blocked", + "record without tree": "1\ngolang.org/x/text v0.14.0 h1:hash=\n\naccess to this site is blocked", + } + for name, page := range pages { + t.Run(name, func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{lookup: testSumDBRecord}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK { + t.Fatalf("lookup: status = %d, want 200", w.Code) + } + db.setFile(lookup, page) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { + t.Errorf("behind a block page: status = %d, body = %q, want the cached record", w.Code, w.Body.String()) + } + + // The page did not replace the record: it is still there in an outage. + db.down.Store(true) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { + t.Errorf("in an outage: status = %d, body = %q, want the cached record", w.Code, w.Body.String()) + } + }) } } @@ -192,7 +428,7 @@ func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { defer moduleUpstream.Close() const lookup = "/lookup/golang.org/x/text@v0.14.0" - db := newFakeSumDB(t, map[string]string{lookup: "lookup record"}) + db := newFakeSumDB(t, map[string]string{lookup: testSumDBRecord}) proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = http.DefaultClient @@ -200,7 +436,7 @@ func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { h.SetSumDBs(map[string]string{testSumDBName: db.server.URL + "/"}) w := serveGoRequest(h, "/sumdb/sum.example"+lookup) - if w.Code != http.StatusOK || w.Body.String() != "lookup record" { + if w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { t.Fatalf("status = %d, body = %q; want 200 with the database's response", w.Code, w.Body.String()) } if n := db.requests(lookup); n != 1 { @@ -213,12 +449,12 @@ func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { func TestGoSumDB_LookupCachedWithinTTL(t *testing.T) { const lookup = "/lookup/golang.org/x/text@v0.14.0" - db := newFakeSumDB(t, map[string]string{lookup: "lookup record"}) + db := newFakeSumDB(t, map[string]string{lookup: testSumDBRecord}) h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) for i := range 2 { w := serveGoRequest(h, "/sumdb/sum.example"+lookup) - if w.Code != http.StatusOK || w.Body.String() != "lookup record" { + if w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { t.Fatalf("request %d: status = %d, body = %q", i, w.Code, w.Body.String()) } } @@ -233,7 +469,7 @@ func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { "/tile/8/data/000": "complete data tile", "/tile/8/0/001.p/5": hashTile(5, "b"), "/tile/8/data/001.p/5": "partial data tile", - "/latest": "signed tree", + "/latest": testSumDBTree, } wantRequests := map[string]int{ "/tile/8/0/000": 1, @@ -289,6 +525,53 @@ func TestGoSumDB_BadTilesAreNotCached(t *testing.T) { } } +// TestGoSumDB_DamagedCachedTileIsFetchedAgain guards the check on the cached +// copy: a complete tile is never revalidated, so a damaged blob would +// otherwise be served for good. The database sends an ETag, so past the TTL +// a request that asks whether the tile changed would get 304 and the damaged +// copy back. +func TestGoSumDB_DamagedCachedTileIsFetchedAgain(t *testing.T) { + const tile = "/tile/8/0/000" + want := hashTile(256, "a") + for _, ttl := range []time.Duration{time.Hour, time.Nanosecond} { + t.Run(fmt.Sprintf("ttl=%s", ttl), func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{tile: want}) + inner := db.server.Config.Handler + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `"tile"`) + if r.Header.Get("If-None-Match") == `"tile"` { + w.WriteHeader(http.StatusNotModified) + return + } + inner.ServeHTTP(w, r) + }) + h := newSumDBTestHandler(t, db.server.URL, true, ttl) + + if w := serveGoRequest(h, "/sumdb/sum.example"+tile); w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + + store := h.proxy.Storage.(*mockStorage) + blob := metadataStoragePath(goEcosystem, goSumDBCacheKey(testSumDBName, db.server.URL, "tile/8/0/000")) + store.mu.Lock() + if _, ok := store.files[blob]; !ok { + store.mu.Unlock() + t.Fatalf("tile not stored at %s", blob) + } + store.files[blob] = []byte(want[:100]) + store.mu.Unlock() + + w := serveGoRequest(h, "/sumdb/sum.example"+tile) + if w.Code != http.StatusOK || w.Body.String() != want { + t.Fatalf("after damage: status = %d, body length %d, want the %d-byte tile", w.Code, w.Body.Len(), len(want)) + } + if n := db.requests(tile); n != 2 { + t.Errorf("upstream requests = %d, want 2: the damaged copy was fetched again", n) + } + }) + } +} + func TestGoSumDB_CompleteTilesNotStoredWithoutMetadataCache(t *testing.T) { const tile = "/tile/8/0/000" db := newFakeSumDB(t, map[string]string{tile: hashTile(256, "a")}) @@ -306,8 +589,8 @@ func TestGoSumDB_CompleteTilesNotStoredWithoutMetadataCache(t *testing.T) { func TestGoSumDB_OfflineFallback(t *testing.T) { files := map[string]string{ - "/lookup/golang.org/x/text@v0.14.0": "lookup record", - "/latest": "signed tree", + "/lookup/golang.org/x/text@v0.14.0": testSumDBRecord, + "/latest": testSumDBTree, "/tile/8/0/000": hashTile(256, "a"), "/tile/8/0/001.p/5": hashTile(5, "b"), } From c7f8e2fc6469444c45393ef76366d8341e1a710a Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 11:31:01 +0200 Subject: [PATCH 08/14] README: say when the go command still goes to the database itself The sentence promised a direct fallback whenever sum.golang.org is unreachable. That's only true once the cached tree has gone stale; a fresh one keeps the proxy answering. configuration.md has the details. --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 30e09a00..2c8b2e24 100644 --- a/README.md +++ b/README.md @@ -205,9 +205,10 @@ Or in your shell profile for persistence. Checksum lookups go through the same URL: the proxy serves `sum.golang.org` at `/go/sumdb/sum.golang.org/`, and the `go` command uses it without further setup, so clients need no route of their own to `sum.golang.org`. If the -proxy cannot reach `https://sum.golang.org`, the `go` command contacts it -directly as before. Other checksum databases are -listed under `upstream.go_sumdbs`, see +proxy loses `https://sum.golang.org`, it keeps answering until its cached +copy of the database's signed tree is older than `metadata_ttl`; after that +the `go` command contacts the database directly as before. Other checksum +databases are listed under `upstream.go_sumdbs`, see [Go checksum database](docs/configuration.md#go-checksum-database). ### Homebrew From 9532c6a94e0e58245e24db47e3e63e45f413477f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 17:55:04 +0200 Subject: [PATCH 09/14] Add upstream.go_sumdbs_offline for networks where only the proxy has egress On a build network where clients cannot reach sum.golang.org at all, the proxy should keep answering for the database during an outage, since a 404 on supported would only send the go command somewhere it cannot go. With the switch set, a cached tree of any age is enough; without it, the tree has to be fresh, as before. That does not make every cached module verify. The go command checks a lookup against tiles of the tree that came with it, and sum.golang.org attaches a different tree to almost every lookup, so it can ask for a tile nobody fetched through the proxy before. configuration.md says so. While the database is known to be unreachable, offline mode also serves cached lookups and tiles straight away. Otherwise each of them would sit through the upstream timeout before the fetch falls back to the cache, and a build would take minutes for as long as the outage lasts. A lookup or tile that falls back to the cache renews what the proxy knows about the database, unless a probe has reached it since the request started. The go command asks supported only once per process, so a long build would otherwise be back to waiting a minute into the outage. recordTimeout is now recordFailure, since lookups use it too. The option needs cache_metadata, since without it there is no cached tree to fall back on, and the config refuses the combination. --- README.md | 5 +- docs/configuration.md | 17 +++ internal/config/config.go | 23 +++- internal/config/config_test.go | 15 +++ internal/handler/go.go | 11 +- internal/handler/go_sumdb.go | 55 ++++++--- internal/handler/go_sumdb_test.go | 187 +++++++++++++++++++++++++----- internal/server/server.go | 2 +- 8 files changed, 259 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 2c8b2e24..3aecba4f 100644 --- a/README.md +++ b/README.md @@ -207,8 +207,9 @@ Checksum lookups go through the same URL: the proxy serves `sum.golang.org` at setup, so clients need no route of their own to `sum.golang.org`. If the proxy loses `https://sum.golang.org`, it keeps answering until its cached copy of the database's signed tree is older than `metadata_ttl`; after that -the `go` command contacts the database directly as before. Other checksum -databases are listed under `upstream.go_sumdbs`, see +the `go` command contacts the database directly as before. Set +`upstream.go_sumdbs_offline` on networks where clients cannot do that. Other +checksum databases are listed under `upstream.go_sumdbs`, see [Go checksum database](docs/configuration.md#go-checksum-database). ### Homebrew diff --git a/docs/configuration.md b/docs/configuration.md index be7df9e6..e0e90476 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -233,6 +233,7 @@ Each upstream used by a built-in package route can be set in YAML or JSON under | `upstream.allow_loopback` | `PROXY_UPSTREAM_ALLOW_LOOPBACK` | `false` | | `upstream.npm` | `PROXY_UPSTREAM_NPM` | `https://registry.npmjs.org` | | `upstream.npm_full_metadata` | `PROXY_UPSTREAM_NPM_FULL_METADATA` | `false` | +| `upstream.go_sumdbs_offline` | `PROXY_UPSTREAM_GO_SUMDBS_OFFLINE` | `false` | | `upstream.cargo` | `PROXY_UPSTREAM_CARGO` | `https://index.crates.io` | | `upstream.cargo_download` | `PROXY_UPSTREAM_CARGO_DOWNLOAD` | `https://static.crates.io/crates` | | `upstream.gem` | `PROXY_UPSTREAM_GEM` | `https://rubygems.org` | @@ -503,6 +504,11 @@ upstream: # Named Go checksum databases, served at /go/sumdb/{name}/. go_sumdbs: sum.golang.org: "https://sum.golang.org" + + # Keep answering for a database from its cached tree, however old, when + # the database cannot be reached. For networks where clients have no + # route to the database of their own. Requires cache_metadata. + go_sumdbs_offline: false ``` The `go` command asks `GOPROXY` for the checksum database before contacting it @@ -520,6 +526,17 @@ clients go to the database themselves, as they would without the proxy. The check waits five seconds at most, and after a failed check the proxy waits a minute before it tries the database again. +Set `upstream.go_sumdbs_offline: true` when clients have no route to the +database of their own. The proxy then keeps answering for it from the cached +tree for as long as the outage lasts and serves cached lookups and tiles. +That does not mean every cached module still verifies: the `go` command +checks each lookup against tiles of the signed tree that came with it, the +database attaches a different tree to almost every lookup, and a tile no +client fetched through the proxy before cannot be served during the outage. +Lookups for modules that were never cached fail as well. Builds whose +`go.sum` already lists every module do not ask the database at all and are +not affected. This requires `cache_metadata`. + With `cache_metadata` enabled, lookups, partial tiles and the latest signed tree follow `metadata_ttl` and are served from cache when the database is unreachable. Complete tiles never change and are not revalidated once cached; diff --git a/internal/config/config.go b/internal/config/config.go index 46cda82a..080209db 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -580,6 +580,14 @@ type UpstreamConfig struct { // Default when empty: {"sum.golang.org": "https://sum.golang.org"}. GoSumDBs map[string]string `json:"go_sumdbs" yaml:"go_sumdbs"` + // GoSumDBsOffline keeps answering for a checksum database from its + // cached signed tree, however old, when the database cannot be reached. + // Set this when clients have no route to the database of their own; + // otherwise the proxy stops answering once the cached tree is older than + // metadata_ttl, and the go command contacts the database directly. + // Requires cache_metadata. Default: false. + GoSumDBsOffline bool `json:"go_sumdbs_offline" yaml:"go_sumdbs_offline"` + // Hex is the upstream Hex repository URL. // Default: https://repo.hex.pm Hex string `json:"hex" yaml:"hex"` @@ -823,6 +831,18 @@ func (u *UpstreamConfig) validateGenericArtifacts() error { return nil } +// validateUpstream checks the upstream block, including the one setting in +// it that depends on a top-level option. +func (c *Config) validateUpstream() error { + if err := c.Upstream.Validate(); err != nil { + return err + } + if c.Upstream.GoSumDBsOffline && !c.CacheMetadata { + return fmt.Errorf("upstream.go_sumdbs_offline requires cache_metadata") + } + return nil +} + // CompileGenericArtifactPattern compiles an upstream.generic_artifacts pattern // so that it only matches a whole path, and checks it has the `version` and // `file` groups. @@ -1150,6 +1170,7 @@ func (c *Config) LoadFromEnv() { setEnvBool(&c.Upstream.AllowLoopback, "PROXY_UPSTREAM_ALLOW_LOOPBACK") setEnvString(&c.Upstream.NPM, "PROXY_UPSTREAM_NPM") setEnvBool(&c.Upstream.NPMFullMetadata, "PROXY_UPSTREAM_NPM_FULL_METADATA") + setEnvBool(&c.Upstream.GoSumDBsOffline, "PROXY_UPSTREAM_GO_SUMDBS_OFFLINE") setEnvString(&c.Upstream.Cargo, "PROXY_UPSTREAM_CARGO") setEnvString(&c.Upstream.CargoDownload, "PROXY_UPSTREAM_CARGO_DOWNLOAD") setEnvString(&c.Upstream.Gem, "PROXY_UPSTREAM_GEM") @@ -1325,7 +1346,7 @@ func (c *Config) validateComponents() error { return err } - if err := c.Upstream.Validate(); err != nil { + if err := c.validateUpstream(); err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 88150ae4..d7b5326c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1638,6 +1638,21 @@ func TestValidateGoSumDBs(t *testing.T) { }, wantErr: true, }, + { + name: "Go checksum database offline mode requires metadata caching", + modify: func(cfg *Config) { + cfg.CacheMetadata = false + cfg.Upstream.GoSumDBsOffline = true + }, + wantErr: true, + }, + { + name: "Go checksum database offline mode with metadata caching", + modify: func(cfg *Config) { + cfg.CacheMetadata = true + cfg.Upstream.GoSumDBsOffline = true + }, + }, } for _, tt := range tests { diff --git a/internal/handler/go.go b/internal/handler/go.go index e5c2f286..a27c37be 100644 --- a/internal/handler/go.go +++ b/internal/handler/go.go @@ -28,11 +28,12 @@ func init() { // GoHandler handles Go module proxy protocol requests. type GoHandler struct { - proxy *Proxy - upstreamURL string - proxyURL string - sumDBs map[string]string - sumDBProbes goSumDBProbes + proxy *Proxy + upstreamURL string + proxyURL string + sumDBs map[string]string + sumDBOffline bool + sumDBProbes goSumDBProbes } // NewGoHandler creates a new Go module proxy handler. diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index 0e0bcbf6..4f53a0a1 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -78,9 +78,10 @@ func (p *goSumDBProbes) record(name string, reachable bool) { p.failed[name] = time.Now() } -// recordTimeout records a failure for a probe started at start, unless the -// fetch it gave up waiting for has succeeded in the meantime. -func (p *goSumDBProbes) recordTimeout(name string, start time.Time) { +// recordFailure records that the database did not answer a request started +// at start, unless a probe has reached it since: a probe that timed out, or +// a lookup that fell back to the cache, must not overwrite a newer success. +func (p *goSumDBProbes) recordFailure(name string, start time.Time) { p.mu.Lock() defer p.mu.Unlock() if !p.succeeded[name].Before(start) { @@ -94,8 +95,11 @@ func (p *goSumDBProbes) recordTimeout(name string, start time.Time) { // SetSumDBs sets the checksum databases served at /sumdb/{name}/, mapping // each database name to its base URL. An empty map keeps the default, -// sum.golang.org. -func (h *GoHandler) SetSumDBs(sumDBs map[string]string) { +// sum.golang.org. offline makes a cached tree of any age enough to keep +// answering for a database the proxy cannot reach, for networks where +// clients have no route to the database of their own. +func (h *GoHandler) SetSumDBs(sumDBs map[string]string, offline bool) { + h.sumDBOffline = offline if len(sumDBs) == 0 { h.sumDBs = map[string]string{defaultGoSumDBName: defaultGoSumDBURL} return @@ -141,7 +145,7 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str if served { return } - h.proxySumDB(w, r, sumDBURL+"/"+path, cacheKey, check, damaged) + h.proxySumDB(w, r, name, sumDBURL+"/"+path, cacheKey, check, damaged) } // sumDBReachable reports whether the proxy can serve a checksum database: it @@ -171,7 +175,7 @@ func (h *GoHandler) sumDBReachable(r *http.Request, name, sumDBURL string) bool case err = <-done: case <-time.After(h.sumDBProbes.probeTimeout()): err = fmt.Errorf("no answer within %s", h.sumDBProbes.probeTimeout()) - h.sumDBProbes.recordTimeout(name, start) + h.sumDBProbes.recordFailure(name, start) case <-r.Context().Done(): // The client went away; that says nothing about the database. return false @@ -204,11 +208,11 @@ func (h *GoHandler) probeSumDB(ctx context.Context, name, cacheKey, sumDBURL str } // cachedTreeServes reports whether a cached tree lets the proxy answer for -// the database although the database did not: only while the tree is fresh, -// so clients fall back to the database once the proxy has lost it. +// the database although the database did not: always with go_sumdbs_offline, +// otherwise only while the tree is fresh. func (h *GoHandler) cachedTreeServes(ctx context.Context, cacheKey string, check func([]byte) error) bool { entry, _, _, ok := h.readCachedSumDB(ctx, cacheKey, check) - return ok && h.treeFresh(entry) + return ok && (h.sumDBOffline || h.treeFresh(entry)) } // treeFresh reports whether a cached tree is within the metadata TTL. @@ -360,15 +364,31 @@ func (h *GoHandler) serveCachedSumDBTile(w http.ResponseWriter, r *http.Request, // go command prints it, and it says why a module version is missing. check, // when set, must accept a response before it is cached or served. damaged // says the cached copy failed check. -func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) { +func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, name, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) { p := h.proxy if !p.CacheMetadata { p.proxyMetadataStream(w, r, upstreamURL, "identity", "*/*") return } + // While the database is known to be unreachable, offline mode serves + // the cached copy at once rather than letting every request wait for + // the upstream timeout before the fetch falls back to it. + if h.sumDBOffline && h.sumDBProbes.recentlyFailed(name) { + if entry, body, contentType, ok := h.readCachedSumDB(r.Context(), cacheKey, check); ok { + p.writeMetadataCachedResponseWithEncoding(w, r, goEcosystem, cacheKey, body, contentType, entry.ContentEncoding.String) + return + } + } - body, contentType, contentEncoding, err := h.fetchSumDB(r.Context(), upstreamURL, cacheKey, check, damaged) - if err != nil { + start := time.Now() + res := h.fetchSumDB(r.Context(), upstreamURL, cacheKey, check, damaged) + if res.stale { + // The database did not answer. Remember it here too: the go command + // asks supported only once per process, so a long build would lose + // the shortcut above a minute into an outage. + h.sumDBProbes.recordFailure(name, start) + } + if err := res.err; err != nil { var notFound *upstreamNotFoundError switch { case errors.As(err, ¬Found) && len(notFound.body) > 0: @@ -385,18 +405,17 @@ func (h *GoHandler) proxySumDB(w http.ResponseWriter, r *http.Request, upstreamU return } - p.writeMetadataCachedResponseWithEncoding(w, r, goEcosystem, cacheKey, body, contentType, contentEncoding) + p.writeMetadataCachedResponseWithEncoding(w, r, goEcosystem, cacheKey, res.body, res.contentType, res.contentEncoding) } // fetchSumDB fetches a checksum database response through the metadata // cache. A cached copy that failed check is fetched again without // revalidating it: past the TTL, a database that sends an ETag would answer // 304 and hand back the damaged copy for good. -func (h *GoHandler) fetchSumDB(ctx context.Context, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) ([]byte, string, string, error) { +func (h *GoHandler) fetchSumDB(ctx context.Context, upstreamURL, cacheKey string, check func([]byte) error, damaged bool) metadataResult { p := h.proxy if damaged { - res := p.fetchMetadataFromUpstream(ctx, goEcosystem, cacheKey, upstreamURL, "*/*", "identity", check, nil) - return res.body, res.contentType, res.contentEncoding, res.err + return p.fetchMetadataFromUpstream(ctx, goEcosystem, cacheKey, upstreamURL, "*/*", "identity", check, nil) } - return p.fetchOrCacheMetadata(ctx, goEcosystem, cacheKey, upstreamURL, "identity", check, "*/*") + return p.lookupMetadata(ctx, goEcosystem, cacheKey, upstreamURL, "*/*", "identity", check) } diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index a849ad83..8bb687c1 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -111,7 +111,7 @@ func newSumDBTestHandler(t *testing.T, sumDBURL string, cacheMetadata bool, ttl proxy.CacheMetadata = cacheMetadata proxy.MetadataTTL = ttl h := NewGoHandler(proxy, "http://proxy.example") - h.SetSumDBs(map[string]string{testSumDBName: sumDBURL}) + h.SetSumDBs(map[string]string{testSumDBName: sumDBURL}, false) return h } @@ -126,7 +126,7 @@ func TestGoSumDB_DefaultsToSumGolangOrg(t *testing.T) { if got := h.sumDBs[defaultGoSumDBName]; got != defaultGoSumDBURL { t.Errorf("default checksum database = %q, want %q", got, defaultGoSumDBURL) } - h.SetSumDBs(nil) + h.SetSumDBs(nil, false) if got := h.sumDBs[defaultGoSumDBName]; got != defaultGoSumDBURL || len(h.sumDBs) != 1 { t.Errorf("SetSumDBs(nil) = %v, want only the default", h.sumDBs) } @@ -192,24 +192,31 @@ func TestGoSumDB_SupportedWithCachedTreeWhileUnreachable(t *testing.T) { } }) - // A stale tree does not count: clients that can reach the database - // themselves should go there. - t.Run("stale tree", func(t *testing.T) { - db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) - h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) - if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { - t.Fatalf("latest: status = %d, want 200", w.Code) - } - db.down.Store(true) - for i := range 2 { // second probe hits the remembered failure - if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { - t.Errorf("probe %d: status = %d, want 404", i, w.Code) + // A stale tree only counts with go_sumdbs_offline. Without it, clients + // that can reach the database themselves should go there. + for _, offline := range []bool{true, false} { + t.Run(fmt.Sprintf("stale tree offline=%v", offline), func(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBOffline = offline + if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { + t.Fatalf("latest: status = %d, want 200", w.Code) } - } - if n := db.requests("/latest"); n != 2 { - t.Errorf("upstream requests = %d, want 2: the failure is remembered", n) - } - }) + db.down.Store(true) + want := http.StatusNotFound + if offline { + want = http.StatusOK + } + for i := range 2 { // second probe hits the remembered failure + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != want { + t.Errorf("probe %d: status = %d, want %d", i, w.Code, want) + } + } + if n := db.requests("/latest"); n != 2 { + t.Errorf("upstream requests = %d, want 2: the failure is remembered", n) + } + }) + } } // TestGoSumDB_SupportedGivesUpOnSilentDatabase: a database that accepts the @@ -237,14 +244,16 @@ func TestGoSumDB_SupportedGivesUpOnSilentDatabase(t *testing.T) { } } -// TestGoSumDB_SupportedWithStaleTreeWhileSilent: a silent database holds -// the fetch past the probe timeout. The stale tree does not count, and the -// second probe must answer at once from the remembered failure. +// TestGoSumDB_SupportedWithStaleTreeWhileSilent: the fetch only falls back +// to a stale cached tree once upstream has failed, which a silent database +// delays past the probe timeout. The proxy can still serve from cache, so +// supported must say so. func TestGoSumDB_SupportedWithStaleTreeWhileSilent(t *testing.T) { release := make(chan struct{}) db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) t.Cleanup(func() { close(release) }) h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBOffline = true h.sumDBProbes.timeout = 50 * time.Millisecond if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { @@ -259,8 +268,8 @@ func TestGoSumDB_SupportedWithStaleTreeWhileSilent(t *testing.T) { go func() { answered <- serveGoRequest(h, "/sumdb/sum.example/supported") }() select { case w := <-answered: - if w.Code != http.StatusNotFound { - t.Fatalf("probe %d: status = %d, want 404", i, w.Code) + if w.Code != http.StatusOK { + t.Fatalf("probe %d: status = %d, want 200 from the cached tree", i, w.Code) } case <-time.After(time.Second): t.Fatalf("probe %d: supported still waiting after a second", i) @@ -306,6 +315,7 @@ func TestGoSumDB_SlowProbeSuccessClearsFailure(t *testing.T) { func TestGoSumDB_StaleFallbackIsNotAnAnswer(t *testing.T) { db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBOffline = true h.sumDBProbes.timeout = 10 * time.Millisecond if w := serveGoRequest(h, "/sumdb/sum.example/latest"); w.Code != http.StatusOK { @@ -318,8 +328,8 @@ func TestGoSumDB_StaleFallbackIsNotAnAnswer(t *testing.T) { }) db.down.Store(true) - if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusNotFound { - t.Fatalf("status = %d, want 404", w.Code) + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 from the cached tree", w.Code) } time.Sleep(300 * time.Millisecond) // the fetch has failed and fallen back by now if !h.sumDBProbes.recentlyFailed(testSumDBName) { @@ -352,15 +362,134 @@ func TestGoSumDB_AnswerCountsWhenCacheWriteFails(t *testing.T) { } } +// TestGoSumDB_OfflineServesCacheWithoutWaiting: in offline mode, once the +// database is known to be unreachable, stale lookups must come from the +// cache at once instead of waiting for the upstream timeout each time. +func TestGoSumDB_OfflineServesCacheWithoutWaiting(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + release := make(chan struct{}) + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree, lookup: testSumDBRecord}) + t.Cleanup(func() { close(release) }) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBOffline = true + h.sumDBProbes.timeout = 50 * time.Millisecond + + for _, path := range []string{"/latest", lookup} { + if w := serveGoRequest(h, "/sumdb/sum.example"+path); w.Code != http.StatusOK { + t.Fatalf("%s: status = %d, want 200", path, w.Code) + } + } + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + }) + + for _, path := range []string{"/supported", "/latest", lookup} { + answered := make(chan *httptest.ResponseRecorder, 1) + go func() { answered <- serveGoRequest(h, "/sumdb/sum.example"+path) }() + select { + case w := <-answered: + if w.Code != http.StatusOK { + t.Errorf("%s: status = %d, want 200 from the cache", path, w.Code) + } + case <-time.After(time.Second): + t.Fatalf("%s: still waiting for the database after a second", path) + } + } +} + +// TestGoSumDB_StaleLookupRemembersFailure: the go command asks supported +// only once per process. A lookup that falls back to the cache must renew +// the remembered failure itself, or offline mode would wait for the upstream +// timeout on every lookup a minute into a long build. +func TestGoSumDB_StaleLookupRemembersFailure(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + db := newFakeSumDB(t, map[string]string{lookup: testSumDBRecord}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + h.sumDBOffline = true + + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK { + t.Fatalf("online: status = %d, want 200", w.Code) + } + if h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Fatal("an answered lookup was recorded as a failure") + } + db.down.Store(true) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { + t.Fatalf("outage: status = %d, body = %q, want the cached record", w.Code, w.Body.String()) + } + if !h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("a lookup served from the cache after the database failed did not remember the failure") + } +} + +// TestGoSumDB_LateStaleLookupKeepsNewerSuccess: a lookup sent while the +// database was failing, which falls back to the cache only after a probe has +// reached the database again, must not mark it unreachable. +func TestGoSumDB_LateStaleLookupKeepsNewerSuccess(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree, lookup: testSumDBRecord}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK { + t.Fatalf("lookup: status = %d, want 200", w.Code) + } + + var sentOnce sync.Once + sent := make(chan struct{}) + release := make(chan struct{}) + inner := db.server.Config.Handler + db.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == lookup { + sentOnce.Do(func() { close(sent) }) + <-release + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + inner.ServeHTTP(w, r) + }) + done := make(chan *httptest.ResponseRecorder, 1) + go func() { done <- serveGoRequest(h, "/sumdb/sum.example"+lookup) }() + <-sent + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Fatalf("supported: status = %d, want 200", w.Code) + } + close(release) + if w := <-done; w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { + t.Fatalf("lookup: status = %d, body = %q, want the cached record", w.Code, w.Body.String()) + } + if h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("a lookup that fell back after the probe succeeded marked the database unreachable") + } +} + +// TestGoSumDB_ShortcutNeedsOfflineMode: without go_sumdbs_offline, a +// remembered failure does not keep cached lookups from asking the database, +// which may well be back. +func TestGoSumDB_ShortcutNeedsOfflineMode(t *testing.T) { + const lookup = "/lookup/golang.org/x/text@v0.14.0" + db := newFakeSumDB(t, map[string]string{lookup: testSumDBRecord}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Nanosecond) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK { + t.Fatalf("lookup: status = %d, want 200", w.Code) + } + + h.sumDBProbes.record(testSumDBName, false) + if w := serveGoRequest(h, "/sumdb/sum.example"+lookup); w.Code != http.StatusOK { + t.Fatalf("lookup with a remembered failure: status = %d, want 200", w.Code) + } + if n := db.requests(lookup); n != 2 { + t.Errorf("database requests = %d, want 2: the cached lookup was served without asking", n) + } +} + func TestGoSumDBProbes_TimeoutDoesNotOverwriteLaterSuccess(t *testing.T) { var probes goSumDBProbes start := time.Now() probes.record(testSumDBName, true) - probes.recordTimeout(testSumDBName, start) + probes.recordFailure(testSumDBName, start) if probes.recentlyFailed(testSumDBName) { t.Error("a success recorded after the probe started was overwritten by its timeout") } - probes.recordTimeout(testSumDBName, time.Now().Add(time.Second)) + probes.recordFailure(testSumDBName, time.Now().Add(time.Second)) if !probes.recentlyFailed(testSumDBName) { t.Error("a timeout with no later success was not recorded") } @@ -433,7 +562,7 @@ func TestGoSumDB_RequestsGoToTheDatabase(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = http.DefaultClient h := NewGoHandlerWithUpstream(proxy, "http://proxy.example", moduleUpstream.URL) - h.SetSumDBs(map[string]string{testSumDBName: db.server.URL + "/"}) + h.SetSumDBs(map[string]string{testSumDBName: db.server.URL + "/"}, false) w := serveGoRequest(h, "/sumdb/sum.example"+lookup) if w.Code != http.StatusOK || w.Body.String() != testSumDBRecord { diff --git a/internal/server/server.go b/internal/server/server.go index b6c93bc8..3ebc3e21 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -402,7 +402,7 @@ func (s *Server) mountProtocolHandlers(r chi.Router, proxy *handler.Proxy) { ) gemHandler := handler.NewGemHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.Gem) goHandler := handler.NewGoHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.Go) - goHandler.SetSumDBs(s.cfg.Upstream.GoSumDBs) + goHandler.SetSumDBs(s.cfg.Upstream.GoSumDBs, s.cfg.Upstream.GoSumDBsOffline) hexHandler := handler.NewHexHandlerWithUpstreams( proxy, s.cfg.BaseURL, From 943009c6692461e58e524d56728ad62302e46bb8 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 22:35:20 +0200 Subject: [PATCH 10/14] Check that a data tile holds W records before caching it. Complete tiles are never fetched again, so a block page cached as one stayed for good. --- internal/handler/go_sumdb.go | 22 ++++++++++++++---- internal/handler/go_sumdb_test.go | 38 +++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index 4f53a0a1..32f2a9c5 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -270,8 +270,8 @@ const goSumDBTreePrefix = "go.sum database tree\n" // followed by a signed tree, which is how the go command reads it; a block // page cached in its place would also replace the copy kept for an outage. // A hash tile holds exactly W hashes, and complete tiles are never -// revalidated, so a bad one would otherwise stay for good. Data tiles have no -// fixed size and only must not be empty. +// revalidated, so a bad one would otherwise stay for good. A data tile holds +// exactly W records. func goSumDBResponseCheck(path string) func([]byte) error { if path == "latest" { return func(body []byte) error { @@ -298,13 +298,27 @@ func goSumDBResponseCheck(path string) func([]byte) error { if tile.L >= 0 && len(body) != tile.W*tlog.HashSize { return fmt.Errorf("checksum database tile %s has %d bytes, want %d", path, len(body), tile.W*tlog.HashSize) } - if len(body) == 0 { - return fmt.Errorf("checksum database tile %s is empty", path) + if tile.L < 0 { + if n := goSumDBDataTileRecords(body); n != tile.W { + return fmt.Errorf("checksum database tile %s has %d records, want %d", path, n, tile.W) + } } return nil } } +// goSumDBDataTileRecords counts the records in a data tile. Each record ends +// with a newline and a blank line separates it from the next; whether one +// follows the last record differs between servers. Anything that does not end +// with a newline counts as no records. +func goSumDBDataTileRecords(body []byte) int { + text, ok := strings.CutSuffix(string(body), "\n") + if !ok || text == "" { + return 0 + } + return strings.Count(text, "\n\n") + 1 +} + // validGoSumDBLookup checks an escaped module@version the way the checksum // database does. Requiring a semantic version also keeps characters such as // '#' and '%' out of the upstream URL. diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index 8bb687c1..98ab7f77 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -32,6 +32,17 @@ func hashTile(width int, fill string) string { return strings.Repeat(fill, width*tlog.HashSize) } +// dataTile returns a data tile body holding width records, laid out the way +// sum.golang.org serves it: a blank line between records, none after the +// last. +func dataTile(width int) string { + records := make([]string, width) + for i := range records { + records[i] = fmt.Sprintf("example.com/m%d v1.0.0 h1:hash=\nexample.com/m%d v1.0.0/go.mod h1:hash=\n", i, i) + } + return strings.Join(records, "\n") +} + // fakeSumDB is a checksum database upstream serving fixed files. Unknown // paths get 404 with the body from notFound, or a default text. type fakeSumDB struct { @@ -481,6 +492,25 @@ func TestGoSumDB_ShortcutNeedsOfflineMode(t *testing.T) { } } +func TestGoSumDBDataTileRecords(t *testing.T) { + tests := []struct { + name string + body string + want int + }{ + {"sum.golang.org layout", dataTile(3), 3}, + {"blank line after the last record", dataTile(3) + "\n", 3}, + {"one record", dataTile(1), 1}, + {"empty", "", 0}, + {"no final newline", "please log in", 0}, + } + for _, tt := range tests { + if got := goSumDBDataTileRecords([]byte(tt.body)); got != tt.want { + t.Errorf("%s: records = %d, want %d", tt.name, got, tt.want) + } + } +} + func TestGoSumDBProbes_TimeoutDoesNotOverwriteLaterSuccess(t *testing.T) { var probes goSumDBProbes start := time.Now() @@ -595,9 +625,9 @@ func TestGoSumDB_LookupCachedWithinTTL(t *testing.T) { func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { files := map[string]string{ "/tile/8/0/000": hashTile(256, "a"), - "/tile/8/data/000": "complete data tile", + "/tile/8/data/000": dataTile(256), "/tile/8/0/001.p/5": hashTile(5, "b"), - "/tile/8/data/001.p/5": "partial data tile", + "/tile/8/data/001.p/5": dataTile(5), "/latest": testSumDBTree, } wantRequests := map[string]int{ @@ -633,6 +663,7 @@ func TestGoSumDB_BadTilesAreNotCached(t *testing.T) { db := newFakeSumDB(t, map[string]string{ tile: "please log in", "/tile/8/data/000.p/3": "", + "/tile/8/data/000": "please log in\n", }) h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) @@ -642,6 +673,9 @@ func TestGoSumDB_BadTilesAreNotCached(t *testing.T) { if w := serveGoRequest(h, "/sumdb/sum.example/tile/8/data/000.p/3"); w.Code != http.StatusBadGateway { t.Errorf("empty data tile: status = %d, want 502", w.Code) } + if w := serveGoRequest(h, "/sumdb/sum.example/tile/8/data/000"); w.Code != http.StatusBadGateway { + t.Errorf("block page as a data tile: status = %d, want 502", w.Code) + } want := hashTile(256, "a") db.setFile(tile, want) From 5332f1f5e0ffb564c6128932e25486b30b67ea22 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 22:35:23 +0200 Subject: [PATCH 11/14] Don't count a tree fresh in the cache as the database answering. The probe never asked it, and during an outage that kept lookups from remembering the failure, so offline mode waited for the upstream timeout on every request until the tree expired. --- internal/handler/go_sumdb.go | 9 +++++++-- internal/handler/go_sumdb_test.go | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index 32f2a9c5..c74b0c8e 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -195,9 +195,14 @@ func (h *GoHandler) sumDBReachable(r *http.Request, name, sumDBURL string) bool } // probeSumDB fetches the database's latest tree and records whether the -// database answered. A tree served from the cache within its TTL counts as -// an answer; one served because upstream failed does not. +// database answered. A tree still fresh in the cache is enough for supported +// but records nothing, since the database was not asked: counting it as an +// answer would keep lookups that fail during an outage from being +// remembered. A tree served because upstream failed is not an answer. func (h *GoHandler) probeSumDB(ctx context.Context, name, cacheKey, sumDBURL string, check func([]byte) error) error { + if entry, _, _, ok := h.readCachedSumDB(ctx, cacheKey, check); ok && h.treeFresh(entry) { + return nil + } res := h.proxy.lookupMetadata(ctx, goEcosystem, cacheKey, sumDBURL+"/latest", "*/*", "identity", check) err := res.err if err == nil && res.stale { diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index 98ab7f77..fa23690a 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -511,6 +511,27 @@ func TestGoSumDBDataTileRecords(t *testing.T) { } } +// TestGoSumDB_FreshTreeIsNotAnAnswer: a supported check answered from a +// fresh cached tree did not reach the database, so it must not cancel a +// failure that a lookup started earlier reports afterwards. +func TestGoSumDB_FreshTreeIsNotAnAnswer(t *testing.T) { + db := newFakeSumDB(t, map[string]string{"/latest": testSumDBTree}) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Fatalf("supported: status = %d, want 200", w.Code) + } + + start := time.Now() + db.down.Store(true) + if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { + t.Fatalf("supported with a fresh cached tree: status = %d, want 200", w.Code) + } + h.sumDBProbes.recordFailure(testSumDBName, start) + if !h.sumDBProbes.recentlyFailed(testSumDBName) { + t.Error("a supported check answered from the cache counted as reaching the database") + } +} + func TestGoSumDBProbes_TimeoutDoesNotOverwriteLaterSuccess(t *testing.T) { var probes goSumDBProbes start := time.Now() From c2ab81031e9752c382ce21d2d230af32c2dd3633 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 22:35:23 +0200 Subject: [PATCH 12/14] Move upstream.go_sumdbs_offline next to upstream.go in the configuration table. --- docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index e0e90476..42a80c01 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -233,11 +233,11 @@ Each upstream used by a built-in package route can be set in YAML or JSON under | `upstream.allow_loopback` | `PROXY_UPSTREAM_ALLOW_LOOPBACK` | `false` | | `upstream.npm` | `PROXY_UPSTREAM_NPM` | `https://registry.npmjs.org` | | `upstream.npm_full_metadata` | `PROXY_UPSTREAM_NPM_FULL_METADATA` | `false` | -| `upstream.go_sumdbs_offline` | `PROXY_UPSTREAM_GO_SUMDBS_OFFLINE` | `false` | | `upstream.cargo` | `PROXY_UPSTREAM_CARGO` | `https://index.crates.io` | | `upstream.cargo_download` | `PROXY_UPSTREAM_CARGO_DOWNLOAD` | `https://static.crates.io/crates` | | `upstream.gem` | `PROXY_UPSTREAM_GEM` | `https://rubygems.org` | | `upstream.go` | `PROXY_UPSTREAM_GO` | `https://proxy.golang.org` | +| `upstream.go_sumdbs_offline` | `PROXY_UPSTREAM_GO_SUMDBS_OFFLINE` | `false` | | `upstream.hex` | `PROXY_UPSTREAM_HEX` | `https://repo.hex.pm` | | `upstream.hex_api` | `PROXY_UPSTREAM_HEX_API` | `https://hex.pm` | | `upstream.pub` | `PROXY_UPSTREAM_PUB` | `https://pub.dev` | From e3cdf73a895969cebc520493c00e97804e2c9256 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 23:08:08 +0200 Subject: [PATCH 13/14] Wait for the clock to move on in the fresh-tree test, since on Windows the first probe's success and the start of the lookup can get the same time. --- internal/handler/go_sumdb_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index fa23690a..1cc3cfe6 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -521,7 +521,15 @@ func TestGoSumDB_FreshTreeIsNotAnAnswer(t *testing.T) { t.Fatalf("supported: status = %d, want 200", w.Code) } + // Coarse clocks (Windows) can give the first probe's success and start + // the same time; start must come after it. + h.sumDBProbes.mu.Lock() + succeeded := h.sumDBProbes.succeeded[testSumDBName] + h.sumDBProbes.mu.Unlock() start := time.Now() + for !start.After(succeeded) { + start = time.Now() + } db.down.Store(true) if w := serveGoRequest(h, "/sumdb/sum.example/supported"); w.Code != http.StatusOK { t.Fatalf("supported with a fresh cached tree: status = %d, want 200", w.Code) From 42df87e035054657feef6cbb77bcd95d48f92fa9 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Tue, 13 Oct 2026 16:23:12 +0200 Subject: [PATCH 14/14] Forward data tiles without storing them. The go command never fetches them, and kept for good they would add up to the whole log; the record count check from before is no longer needed. --- docs/configuration.md | 10 +++-- internal/handler/go_sumdb.go | 29 ++++-------- internal/handler/go_sumdb_test.go | 74 +++++++++++++++---------------- 3 files changed, 51 insertions(+), 62 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 42a80c01..2ff1ab5c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -537,10 +537,12 @@ Lookups for modules that were never cached fail as well. Builds whose `go.sum` already lists every module do not ask the database at all and are not affected. This requires `cache_metadata`. -With `cache_metadata` enabled, lookups, partial tiles and the latest signed -tree follow `metadata_ttl` and are served from cache when the database is -unreachable. Complete tiles never change and are not revalidated once cached; -a tile of the wrong size is refused and not stored. +With `cache_metadata` enabled, lookups, partial hash tiles and the latest +signed tree follow `metadata_ttl` and are served from cache when the database +is unreachable. Complete hash tiles never change and are not revalidated once +cached; a tile of the wrong size is refused and not stored. Data tiles are +always forwarded and never stored: the `go` command does not fetch them, only +mirrors and auditors do. Without `cache_metadata` the requests are forwarded and nothing is stored. A 404 from the database keeps its text, which the `go` command prints to diff --git a/internal/handler/go_sumdb.go b/internal/handler/go_sumdb.go index c74b0c8e..020a126e 100644 --- a/internal/handler/go_sumdb.go +++ b/internal/handler/go_sumdb.go @@ -135,6 +135,12 @@ func (h *GoHandler) handleSumDB(w http.ResponseWriter, r *http.Request, rest str http.NotFound(w, r) return } + // Only mirrors and auditors fetch data tiles, never the go command, and + // together they hold the whole log, so they are not stored. + if tile, err := tlog.ParseTilePath(path); err == nil && tile.L < 0 { + h.proxy.proxyMetadataStream(w, r, sumDBURL+"/"+path, "identity", "*/*") + return + } cacheKey := goSumDBCacheKey(name, sumDBURL, path) check := goSumDBResponseCheck(path) @@ -275,8 +281,8 @@ const goSumDBTreePrefix = "go.sum database tree\n" // followed by a signed tree, which is how the go command reads it; a block // page cached in its place would also replace the copy kept for an outage. // A hash tile holds exactly W hashes, and complete tiles are never -// revalidated, so a bad one would otherwise stay for good. A data tile holds -// exactly W records. +// revalidated, so a bad one would otherwise stay for good. Data tiles are +// never cached and not checked. func goSumDBResponseCheck(path string) func([]byte) error { if path == "latest" { return func(body []byte) error { @@ -300,30 +306,13 @@ func goSumDBResponseCheck(path string) func([]byte) error { return nil } return func(body []byte) error { - if tile.L >= 0 && len(body) != tile.W*tlog.HashSize { + if len(body) != tile.W*tlog.HashSize { return fmt.Errorf("checksum database tile %s has %d bytes, want %d", path, len(body), tile.W*tlog.HashSize) } - if tile.L < 0 { - if n := goSumDBDataTileRecords(body); n != tile.W { - return fmt.Errorf("checksum database tile %s has %d records, want %d", path, n, tile.W) - } - } return nil } } -// goSumDBDataTileRecords counts the records in a data tile. Each record ends -// with a newline and a blank line separates it from the next; whether one -// follows the last record differs between servers. Anything that does not end -// with a newline counts as no records. -func goSumDBDataTileRecords(body []byte) int { - text, ok := strings.CutSuffix(string(body), "\n") - if !ok || text == "" { - return 0 - } - return strings.Count(text, "\n\n") + 1 -} - // validGoSumDBLookup checks an escaped module@version the way the checksum // database does. Requiring a semantic version also keeps characters such as // '#' and '%' out of the upstream URL. diff --git a/internal/handler/go_sumdb_test.go b/internal/handler/go_sumdb_test.go index 1cc3cfe6..6a0c47b4 100644 --- a/internal/handler/go_sumdb_test.go +++ b/internal/handler/go_sumdb_test.go @@ -492,25 +492,6 @@ func TestGoSumDB_ShortcutNeedsOfflineMode(t *testing.T) { } } -func TestGoSumDBDataTileRecords(t *testing.T) { - tests := []struct { - name string - body string - want int - }{ - {"sum.golang.org layout", dataTile(3), 3}, - {"blank line after the last record", dataTile(3) + "\n", 3}, - {"one record", dataTile(1), 1}, - {"empty", "", 0}, - {"no final newline", "please log in", 0}, - } - for _, tt := range tests { - if got := goSumDBDataTileRecords([]byte(tt.body)); got != tt.want { - t.Errorf("%s: records = %d, want %d", tt.name, got, tt.want) - } - } -} - // TestGoSumDB_FreshTreeIsNotAnAnswer: a supported check answered from a // fresh cached tree did not reach the database, so it must not cancel a // failure that a lookup started earlier reports afterwards. @@ -653,18 +634,14 @@ func TestGoSumDB_LookupCachedWithinTTL(t *testing.T) { func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { files := map[string]string{ - "/tile/8/0/000": hashTile(256, "a"), - "/tile/8/data/000": dataTile(256), - "/tile/8/0/001.p/5": hashTile(5, "b"), - "/tile/8/data/001.p/5": dataTile(5), - "/latest": testSumDBTree, + "/tile/8/0/000": hashTile(256, "a"), + "/tile/8/0/001.p/5": hashTile(5, "b"), + "/latest": testSumDBTree, } wantRequests := map[string]int{ - "/tile/8/0/000": 1, - "/tile/8/data/000": 1, - "/tile/8/0/001.p/5": 2, - "/tile/8/data/001.p/5": 2, - "/latest": 2, + "/tile/8/0/000": 1, + "/tile/8/0/001.p/5": 2, + "/latest": 2, } db := newFakeSumDB(t, files) // Every cached entry is past its TTL by the second request. @@ -685,26 +662,47 @@ func TestGoSumDB_CompleteTilesSkipRevalidation(t *testing.T) { } } +// TestGoSumDB_DataTilesAreNotStored: the go command never fetches data +// tiles, and stored they would add up to the whole log. +func TestGoSumDB_DataTilesAreNotStored(t *testing.T) { + files := map[string]string{ + "/tile/8/data/000": dataTile(256), + "/tile/8/data/001.p/5": dataTile(5), + } + db := newFakeSumDB(t, files) + h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) + + for range 2 { + for path, want := range files { + w := serveGoRequest(h, "/sumdb/sum.example"+path) + if w.Code != http.StatusOK || w.Body.String() != want { + t.Fatalf("%s: status = %d, body length %d, want the %d-byte tile", path, w.Code, w.Body.Len(), len(want)) + } + } + } + for path := range files { + if n := db.requests(path); n != 2 { + t.Errorf("%s: database requests = %d, want 2", path, n) + } + key := goSumDBCacheKey(testSumDBName, db.server.URL, strings.TrimPrefix(path, "/")) + if entry, _ := h.proxy.DB.GetMetadataCache(goEcosystem, key); entry != nil { + t.Errorf("%s: stored in the metadata cache", path) + } + } +} + // TestGoSumDB_BadTilesAreNotCached guards complete tiles, which are never // revalidated once cached: a response of the wrong size must not be stored. func TestGoSumDB_BadTilesAreNotCached(t *testing.T) { const tile = "/tile/8/0/000" db := newFakeSumDB(t, map[string]string{ - tile: "please log in", - "/tile/8/data/000.p/3": "", - "/tile/8/data/000": "please log in\n", + tile: "please log in", }) h := newSumDBTestHandler(t, db.server.URL, true, time.Hour) if w := serveGoRequest(h, "/sumdb/sum.example"+tile); w.Code != http.StatusBadGateway { t.Fatalf("wrong-size tile: status = %d, want 502", w.Code) } - if w := serveGoRequest(h, "/sumdb/sum.example/tile/8/data/000.p/3"); w.Code != http.StatusBadGateway { - t.Errorf("empty data tile: status = %d, want 502", w.Code) - } - if w := serveGoRequest(h, "/sumdb/sum.example/tile/8/data/000"); w.Code != http.StatusBadGateway { - t.Errorf("block page as a data tile: status = %d, want 502", w.Code) - } want := hashTile(256, "a") db.setFile(tile, want)