Merge pull request #79 from git-pkgs/fix-metadata-size-limit

Fix silent truncation of large npm metadata responses

Author: andrew

internal/handler/handler.go

@@ -4,6 +4,7 @@ package handler
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -32,15 +33,26 @@ func containsPathTraversal(path string) bool {
 
 const defaultHTTPTimeout = 30 * time.Second
 
-// maxMetadataSize is the maximum size of upstream metadata responses (50 MB).
+// maxMetadataSize is the maximum size of upstream metadata responses (100 MB).
 // Package metadata (e.g. npm with many versions) can be large, but unbounded
 // reads risk OOM if an upstream misbehaves.
-const maxMetadataSize = 50 << 20
+const maxMetadataSize = 100 << 20
+
+// ErrMetadataTooLarge is returned when upstream metadata exceeds maxMetadataSize.
+var ErrMetadataTooLarge = errors.New("metadata response exceeds size limit")
 
 // ReadMetadata reads an upstream response body with a size limit to prevent OOM
-// from unexpectedly large responses.
+// from unexpectedly large responses. Returns ErrMetadataTooLarge if the response
+// is truncated by the limit.
 func ReadMetadata(r io.Reader) ([]byte, error) {
-	return io.ReadAll(io.LimitReader(r, maxMetadataSize))
+	data, err := io.ReadAll(io.LimitReader(r, maxMetadataSize+1))
+	if err != nil {
+		return nil, err
+	}
+	if int64(len(data)) > maxMetadataSize {
+		return nil, ErrMetadataTooLarge
+	}
+	return data, nil
 }
 
 // Proxy provides shared functionality for protocol handlers.

internal/handler/npm.go

@@ -73,7 +73,13 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques
 		JSONError(w, http.StatusInternalServerError, "failed to create request")
 		return
 	}
-	req.Header.Set("Accept", "application/json")
+	// Use abbreviated metadata when cooldown is disabled — it's much smaller
+	// (e.g. drizzle-orm: 4MB vs 92MB) but lacks the time map needed for cooldown.
+	if h.proxy.Cooldown != nil && h.proxy.Cooldown.Enabled() {
+		req.Header.Set("Accept", "application/json")
+	} else {
+		req.Header.Set("Accept", "application/vnd.npm.install-v1+json")
+	}
 
 	resp, err := h.proxy.HTTPClient.Do(req)
 	if err != nil {

internal/handler/npm_test.go

@@ -293,6 +293,62 @@ func TestNPMRewriteMetadataCooldownExemptPackage(t *testing.T) {
 	}
 }
 
+func TestNPMHandlerUsesAbbreviatedMetadata(t *testing.T) {
+	var gotAccept string
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotAccept = r.Header.Get("Accept")
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{
+			"name": "testpkg",
+			"versions": {
+				"1.0.0": {
+					"name": "testpkg",
+					"version": "1.0.0",
+					"dist": {
+						"tarball": "https://registry.npmjs.org/testpkg/-/testpkg-1.0.0.tgz"
+					}
+				}
+			}
+		}`))
+	}))
+	defer upstream.Close()
+
+	t.Run("no cooldown uses abbreviated metadata", func(t *testing.T) {
+		h := &NPMHandler{
+			proxy:       testProxy(),
+			upstreamURL: upstream.URL,
+			proxyURL:    "http://proxy.local",
+		}
+
+		req := httptest.NewRequest(http.MethodGet, "/testpkg", nil)
+		w := httptest.NewRecorder()
+		h.handlePackageMetadata(w, req)
+
+		if gotAccept != "application/vnd.npm.install-v1+json" {
+			t.Errorf("Accept = %q, want abbreviated metadata header", gotAccept)
+		}
+	})
+
+	t.Run("cooldown enabled uses full metadata", func(t *testing.T) {
+		proxy := testProxy()
+		proxy.Cooldown = &cooldown.Config{Default: "3d"}
+
+		h := &NPMHandler{
+			proxy:       proxy,
+			upstreamURL: upstream.URL,
+			proxyURL:    "http://proxy.local",
+		}
+
+		req := httptest.NewRequest(http.MethodGet, "/testpkg", nil)
+		w := httptest.NewRecorder()
+		h.handlePackageMetadata(w, req)
+
+		if gotAccept == "application/vnd.npm.install-v1+json" {
+			t.Error("cooldown enabled should use full metadata, not abbreviated")
+		}
+	})
+}
+
 func TestNPMHandlerMetadataNotFound(t *testing.T) {
 	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)

internal/handler/read_metadata_test.go

@@ -2,6 +2,7 @@ package handler
 
 import (
 	"bytes"
+	"errors"
 	"testing"
 )
 
@@ -17,9 +18,8 @@ func TestReadMetadata(t *testing.T) {
 		}
 	})
 
-	t.Run("truncates at limit", func(t *testing.T) {
-		// Create a reader slightly larger than maxMetadataSize
-		data := make([]byte, maxMetadataSize+100)
+	t.Run("exactly at limit", func(t *testing.T) {
+		data := make([]byte, maxMetadataSize)
 		for i := range data {
 			data[i] = 'x'
 		}
@@ -31,4 +31,15 @@ func TestReadMetadata(t *testing.T) {
 			t.Errorf("got length %d, want %d", len(got), maxMetadataSize)
 		}
 	})
+
+	t.Run("over limit returns error", func(t *testing.T) {
+		data := make([]byte, maxMetadataSize+100)
+		for i := range data {
+			data[i] = 'x'
+		}
+		_, err := ReadMetadata(bytes.NewReader(data))
+		if !errors.Is(err, ErrMetadataTooLarge) {
+			t.Errorf("got error %v, want ErrMetadataTooLarge", err)
+		}
+	})
 }