Pass upstream 404 and auth failures through, and flag stale npm metadata

internal/handler/handler.go

@@ -1204,6 +1204,17 @@ var ErrUpstreamNotFound = fmt.Errorf("upstream: %w", fetch.ErrNotFound)
 // upstreamNotFoundError.
 const upstreamNotFoundBodyLimit = 4 << 10
 
+// upstreamStatusError is a definitive answer from upstream, not a transport
+// failure or a retryable status. npm relays it instead of serving the cached
+// copy.
+type upstreamStatusError struct {
+	status int
+}
+
+func (e *upstreamStatusError) Error() string {
+	return fmt.Sprintf("upstream returned %d", e.status)
+}
+
 // upstreamNotFoundError is ErrUpstreamNotFound with the start of the
 // upstream's 404 body, for handlers that relay the reason to clients.
 type upstreamNotFoundError struct {
@@ -1391,6 +1402,16 @@ func (p *Proxy) fetchMetadataFromUpstream(ctx context.Context, ecosystem, cacheK
 		return metadataResult{body: meta.body, contentType: meta.contentType, contentEncoding: meta.contentEncoding, digest: digest}
 	}
 
+	// npm relays a definitive answer from the registry: serving the cached
+	// copy would hide an unpublished package (404) or credentials it rejected
+	// (401/403). Other ecosystems keep their documented stale fallback, and
+	// transport failures and retryable statuses (429, 5xx) fall back for npm
+	// too.
+	var upstreamStatus *upstreamStatusError
+	if ecosystem == "npm" && (errors.Is(err, ErrUpstreamNotFound) || errors.As(err, &upstreamStatus)) {
+		return metadataResult{err: err}
+	}
+
 	// Upstream failed -- fall back to cache if available
 	if !p.CacheMetadata || entry == nil {
 		return metadataResult{err: fmt.Errorf("upstream failed and no cached metadata: %w", err)}
@@ -1607,6 +1628,12 @@ func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, e
 		return nil, &upstreamNotFoundError{body: body}
 	}
 	if resp.StatusCode != http.StatusOK {
+		// 401 and 403 are the upstream answering, not failing: record them
+		// as definitive so npm can relay them instead of serving the cached
+		// copy.
+		if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
+			return nil, &upstreamStatusError{status: resp.StatusCode}
+		}
 		return nil, fmt.Errorf("upstream returned %d", resp.StatusCode)
 	}
 

internal/handler/npm.go

@@ -201,6 +201,7 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques
 	h.proxy.Logger.Info("npm metadata request", "package", packageName, "path", packagePath)
 
 	registryURL, cacheKey := h.sourceFor(packageName)
+	_, routed := h.routes.match(packageName)
 	// A version or dist-tag is its own path segment. Escaping the whole path
 	// turns "lodash/4.17.21" into "lodash%2F4.17.21", which the registry
 	// rejects with 405.
@@ -226,15 +227,7 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques
 
 	upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept)
 	if err := upstream.err; err != nil {
-		if errors.Is(err, ErrUpstreamNotFound) {
-			if versionLookup {
-				JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error())
-			} else {
-				JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error())
-			}
-			return
-		}
-		if writeUpstreamHeld(w, err) {
+		if h.writeNPMMetadataError(w, err, versionLookup, routed) {
 			return
 		}
 		h.proxy.Logger.Error("failed to fetch npm metadata", "error", err)
@@ -269,9 +262,34 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
+	rewritten.stale = upstream.stale
 	serveRewrittenMetadata(w, r, rewritten)
 }
 
+// writeNPMMetadataError writes the response for a failed npm metadata lookup
+// and reports whether it did. When it reports false the caller answers the
+// upstream it could not reach with 502.
+func (h *NPMHandler) writeNPMMetadataError(w http.ResponseWriter, err error, versionLookup, routed bool) bool {
+	if errors.Is(err, ErrUpstreamNotFound) {
+		if versionLookup {
+			JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error())
+		} else {
+			JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error())
+		}
+		return true
+	}
+	// A route's own credentials are the proxy's business, not the client's:
+	// relaying its 401 would send npm off authenticating to the proxy. The
+	// default registry's answer is relayed instead, so broken upstream
+	// credentials fail loudly rather than serving a stale 200.
+	var upstreamStatus *upstreamStatusError
+	if errors.As(err, &upstreamStatus) && !routed {
+		JSONError(w, upstreamStatus.status, "upstream rejected the request")
+		return true
+	}
+	return writeUpstreamHeld(w, err)
+}
+
 // packumentAccept returns the Accept header packuments are requested with.
 // It prefers the smaller abbreviated packument format but includes
 // application/json as a fallback so upstreams that reject the abbreviated type

internal/handler/npm_metadata_stale_test.go

@@ -0,0 +1,199 @@
+package handler
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+// testPackument is a minimal npm packument the proxy can rewrite.
+const testPackument = `{"name":"testpkg","versions":{"1.0.0":{"name":"testpkg","version":"1.0.0","dist":{"tarball":"https://registry.npmjs.org/testpkg/-/testpkg-1.0.0.tgz"}}}}`
+
+// newStaleMetadataUpstream serves testPackument until status holds a non-zero
+// HTTP status, which it answers instead.
+func newStaleMetadataUpstream(t *testing.T) (*httptest.Server, *atomic.Int32) {
+	t.Helper()
+	var status atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		if code := status.Load(); code != 0 {
+			w.WriteHeader(int(code))
+			return
+		}
+		w.Header().Set("Content-Type", contentTypeJSON)
+		_, _ = io.WriteString(w, testPackument)
+	}))
+	t.Cleanup(server.Close)
+	return server, &status
+}
+
+// newStaleMetadataHandler returns an npm handler over an expiring metadata
+// cache, with the rewrite cache enabled when rewriteCache is true.
+func newStaleMetadataHandler(t *testing.T, upstreamURL string, rewriteCache bool) *NPMHandler {
+	t.Helper()
+	proxy, _, _, _ := setupTestProxy(t)
+	proxy.CacheMetadata = true
+	proxy.MetadataTTL = time.Millisecond
+	proxy.HTTPClient = http.DefaultClient
+	if rewriteCache {
+		proxy.SetMetadataRewriteCacheSize(1 << 20)
+	}
+	return NewNPMHandler(proxy, "http://proxy.test", upstreamURL)
+}
+
+// primeStaleMetadataCache fetches the packument once and lets the entry expire,
+// so the next request goes upstream again.
+func primeStaleMetadataCache(t *testing.T, h *NPMHandler) string {
+	t.Helper()
+	w := serveNPM(t, h, http.MethodGet, "/testpkg", nil, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("priming request: status = %d, want 200; body: %s", w.Code, w.Body.String())
+	}
+	time.Sleep(5 * time.Millisecond)
+	return w.Body.String()
+}
+
+func TestNPMMetadataStaleWarningAfterUpstreamFailure(t *testing.T) {
+	for _, rewriteCache := range []bool{false, true} {
+		t.Run(fmt.Sprintf("rewrite cache=%v", rewriteCache), func(t *testing.T) {
+			upstream, status := newStaleMetadataUpstream(t)
+			h := newStaleMetadataHandler(t, upstream.URL, rewriteCache)
+			primed := primeStaleMetadataCache(t, h)
+
+			status.Store(http.StatusBadGateway)
+			w := serveNPM(t, h, http.MethodGet, "/testpkg", nil, nil)
+
+			if w.Code != http.StatusOK {
+				t.Fatalf("stale request: status = %d, want 200; body: %s", w.Code, w.Body.String())
+			}
+			if w.Body.String() != primed {
+				t.Errorf("stale body = %q, want the cached %q", w.Body.String(), primed)
+			}
+			if got := w.Header().Get("Warning"); got != `110 - "Response is Stale"` {
+				t.Errorf("Warning = %q, want the stale warning", got)
+			}
+		})
+	}
+}
+
+func TestNPMMetadataFreshResponseHasNoStaleWarning(t *testing.T) {
+	upstream, _ := newStaleMetadataUpstream(t)
+	h := newStaleMetadataHandler(t, upstream.URL, true)
+	primed := primeStaleMetadataCache(t, h)
+
+	w := serveNPM(t, h, http.MethodGet, "/testpkg", nil, nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String())
+	}
+	if w.Body.String() != primed {
+		t.Errorf("body = %q, want %q", w.Body.String(), primed)
+	}
+	if got := w.Header().Get("Warning"); got != "" {
+		t.Errorf("Warning = %q, want none on a fresh response", got)
+	}
+}
+
+// TestNPMMetadataPassesDefinitiveUpstreamAnswersThrough checks that an
+// unpublished package or rejected credentials reach the client instead of the
+// cached copy.
+func TestNPMMetadataPassesDefinitiveUpstreamAnswersThrough(t *testing.T) {
+	for _, code := range []int{http.StatusNotFound, http.StatusUnauthorized, http.StatusForbidden} {
+		t.Run(http.StatusText(code), func(t *testing.T) {
+			upstream, status := newStaleMetadataUpstream(t)
+			h := newStaleMetadataHandler(t, upstream.URL, true)
+			primed := primeStaleMetadataCache(t, h)
+
+			status.Store(int32(code))
+			w := serveNPM(t, h, http.MethodGet, "/testpkg", nil, nil)
+
+			if w.Code != code {
+				t.Fatalf("status = %d, want %d; body: %s", w.Code, code, w.Body.String())
+			}
+			if w.Body.String() == primed {
+				t.Error("the cached packument was served for a definitive upstream answer")
+			}
+			if got := w.Header().Get("Warning"); got != "" {
+				t.Errorf("Warning = %q, want none", got)
+			}
+			var body map[string]any
+			if err := json.NewDecoder(strings.NewReader(w.Body.String())).Decode(&body); err != nil {
+				t.Fatalf("response body is not JSON: %v", err)
+			}
+			if _, ok := body["error"]; !ok {
+				t.Errorf("response body %v missing \"error\" key", body)
+			}
+		})
+	}
+}
+
+// TestNPMMetadataServesStaleOnRetryableStatus guards the offline fallback for
+// rate limits and outages, which must still serve the cached copy.
+func TestNPMMetadataServesStaleOnRetryableStatus(t *testing.T) {
+	upstream, status := newStaleMetadataUpstream(t)
+	h := newStaleMetadataHandler(t, upstream.URL, true)
+	primed := primeStaleMetadataCache(t, h)
+
+	status.Store(http.StatusServiceUnavailable)
+	w := serveNPM(t, h, http.MethodGet, "/testpkg", nil, nil)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String())
+	}
+	if w.Body.String() != primed {
+		t.Errorf("body = %q, want the cached %q", w.Body.String(), primed)
+	}
+	if got := w.Header().Get("Warning"); got != `110 - "Response is Stale"` {
+		t.Errorf("Warning = %q, want the stale warning", got)
+	}
+}
+
+// TestGenericMetadataKeepsStaleFallback guards the documented fallback for
+// ecosystems other than npm (#486): a cached document is still served when
+// upstream answers 404/401/403. Only npm relays those answers.
+func TestGenericMetadataKeepsStaleFallback(t *testing.T) {
+	for _, code := range []int{http.StatusNotFound, http.StatusUnauthorized, http.StatusForbidden} {
+		t.Run(http.StatusText(code), func(t *testing.T) {
+			upstream, status := newStaleMetadataUpstream(t)
+
+			proxy, _, _, _ := setupTestProxy(t)
+			proxy.CacheMetadata = true
+			proxy.MetadataTTL = time.Millisecond
+			proxy.HTTPClient = http.DefaultClient
+
+			upstreamURL := upstream.URL + "/testpkg"
+			primed := primeGenericMetadataCache(t, proxy, upstreamURL)
+			time.Sleep(5 * time.Millisecond)
+
+			status.Store(int32(code))
+			w := httptest.NewRecorder()
+			proxy.ProxyCached(w, httptest.NewRequest(http.MethodGet, "/testpkg", nil), upstreamURL, "composer", "testpkg")
+
+			if w.Code != http.StatusOK {
+				t.Fatalf("status = %d, want 200 from the cached copy; body: %s", w.Code, w.Body.String())
+			}
+			if w.Body.String() != primed {
+				t.Errorf("body = %q, want the cached %q", w.Body.String(), primed)
+			}
+			if got := w.Header().Get("Warning"); got != `110 - "Response is Stale"` {
+				t.Errorf("Warning = %q, want the stale warning", got)
+			}
+		})
+	}
+}
+
+// primeGenericMetadataCache fetches a metadata document once and returns the
+// body, so a later request can be compared against it.
+func primeGenericMetadataCache(t *testing.T, proxy *Proxy, upstreamURL string) string {
+	t.Helper()
+	w := httptest.NewRecorder()
+	proxy.ProxyCached(w, httptest.NewRequest(http.MethodGet, "/testpkg", nil), upstreamURL, "composer", "testpkg")
+	if w.Code != http.StatusOK {
+		t.Fatalf("priming request: status = %d, want 200; body: %s", w.Code, w.Body.String())
+	}
+	return w.Body.String()
+}

internal/handler/rewrite_cache.go

@@ -81,11 +81,13 @@ type metadataValidators struct {
 
 // rewrittenMetadata is a rewritten metadata document with its validators.
 // gzipped is the document gzip-compressed, or nil when there is no
-// compressed copy to serve.
+// compressed copy to serve. stale is set when the body is the cached copy
+// served because upstream failed.
 type rewrittenMetadata struct {
 	body []byte
 	metadataValidators
 	gzipped []byte
+	stale   bool
 }
 
 // metadataETag returns the strong ETag of a rewritten document.
@@ -351,6 +353,9 @@ func serveRewrittenMetadata(w http.ResponseWriter, r *http.Request, m rewrittenM
 		w.WriteHeader(http.StatusNotModified)
 		return
 	}
+	if m.stale {
+		w.Header().Set("Warning", `110 - "Response is Stale"`)
+	}
 	w.Header().Set(headerContentType, contentTypeJSON)
 	if compressed {
 		w.Header().Set(headerContentEncoding, "gzip")