From 34985b54d9567181129a2cc961f3c2dc37b842ab Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 4 Oct 2026 18:05:42 +0100 Subject: [PATCH] Support npm content-addressed tarball URLs --- internal/handler/npm.go | 95 ++++++++++++++--- internal/handler/npm_tarball_test.go | 154 +++++++++++++++++++++++++++ internal/handler/npm_test.go | 28 ++--- 3 files changed, 240 insertions(+), 37 deletions(-) create mode 100644 internal/handler/npm_tarball_test.go diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 663c8c66..98ae10a3 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -10,6 +10,7 @@ import ( "net/url" "sort" "strings" + "sync" "time" ) @@ -315,6 +316,13 @@ func (h *NPMHandler) rewriteTarballURLs(versions map[string]any, packageName str if idx := strings.LastIndex(tarball, "/"); idx >= 0 { filename = tarball[idx+1:] } + if h.extractVersionFromFilename(packageName, filename) != version { + _, shortName, scoped := strings.Cut(packageName, "/") + if !scoped { + shortName = packageName + } + filename = shortName + "-" + version + ".tgz" + } escapedName := url.PathEscape(packageName) newTarball := fmt.Sprintf("%s/npm/%s/-/%s", h.proxyURL, escapedName, filename) @@ -377,22 +385,23 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { h.proxy.Logger.Info("npm download request", "package", packageName, "version", version, "filename", filename) - if h.versionInCooldown(r, packageName, version) { + if h.proxy.versionDenied("npm", packageName, version) { + JSONError(w, http.StatusForbidden, ErrVersionDenied.Error()+": "+canonicalVersionPURL("npm", packageName, version)) + return + } + metadata := sync.OnceValues(func() ([]byte, error) { + upstreamURL := h.upstreamURL + "/" + url.PathEscape(packageName) + body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, upstreamURL, contentTypeJSON) + return body, err + }) + if h.versionInCooldown(packageName, version, metadata) { h.proxy.Logger.Info("cooldown: withholding npm tarball", "package", packageName, "version", version) JSONError(w, http.StatusNotFound, "version not found") return } - downloadURL := fmt.Sprintf( - "%s/%s/-/%s", - h.upstreamURL, - escapeNPMDownloadPackage(packageName), - url.PathEscape(filename), - ) - result, err := h.proxy.GetOrFetchArtifactFromURL( - r.Context(), "npm", packageName, version, filename, downloadURL, - ) + result, err := h.getTarball(r, packageName, version, filename, metadata) if err != nil { switch { case errors.Is(err, ErrUpstreamNotFound): @@ -409,6 +418,60 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { ServeArtifactRequest(w, r, result) } +func (h *NPMHandler) getTarball(r *http.Request, packageName, version, filename string, metadata func() ([]byte, error)) (*CacheResult, error) { + if cached, err := h.proxy.GetCachedArtifact(r.Context(), "npm", packageName, version, filename); err != nil || cached != nil { + return cached, err + } + downloadURL := fmt.Sprintf("%s/%s/-/%s", h.upstreamURL, escapeNPMDownloadPackage(packageName), url.PathEscape(filename)) + body, err := metadata() + if err == nil { + if tarball := npmVersionTarball(body, version); tarball != "" { + downloadURL, err = h.validateTarballURL(tarball) + if err != nil { + return nil, err + } + } + } + return h.proxy.GetOrFetchArtifactFromURL(r.Context(), "npm", packageName, version, filename, downloadURL) +} + +func npmVersionTarball(body []byte, version string) string { + var metadata struct { + Versions map[string]struct { + Dist struct { + Tarball string `json:"tarball"` + } `json:"dist"` + } `json:"versions"` + } + if err := json.Unmarshal(body, &metadata); err != nil { + return "" + } + return metadata.Versions[version].Dist.Tarball +} + +func (h *NPMHandler) validateTarballURL(raw string) (string, error) { + tarball, err := url.Parse(raw) + if err != nil { + return "", fmt.Errorf("parsing npm tarball URL: %w", err) + } + upstream, err := url.Parse(h.upstreamURL) + if err != nil { + return "", fmt.Errorf("parsing npm upstream URL: %w", err) + } + if tarball.User != nil || tarball.Fragment != "" || + tarball.Scheme != upstream.Scheme || !strings.EqualFold(tarball.Host, upstream.Host) { + return "", errors.New("npm tarball URL does not match upstream registry") + } + if containsPathTraversal(tarball.Path) || strings.Contains(tarball.Path, "\\") { + return "", errors.New("npm tarball URL contains path traversal") + } + basePath := strings.TrimRight(upstream.Path, "/") + if basePath != "" && !strings.HasPrefix(tarball.Path, basePath+"/") { + return "", errors.New("npm tarball URL is outside upstream base path") + } + return tarball.String(), nil +} + // versionInCooldown reports whether a version is still inside the cooldown // window. Filtering the packument is not enough on its own: tarball URLs are // predictable and lockfiles record them directly, so `npm ci` reaches the @@ -420,7 +483,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { // fetched and parsed at most once per version. A version with no usable // publish time is allowed through, matching how applyCooldownFiltering // treats it. -func (h *NPMHandler) versionInCooldown(r *http.Request, packageName, version string) bool { +func (h *NPMHandler) versionInCooldown(packageName, version string, metadata func() ([]byte, error)) bool { if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { return false } @@ -430,25 +493,23 @@ func (h *NPMHandler) versionInCooldown(r *http.Request, packageName, version str return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), ver.PublishedAt.Time) } - upstreamURL := fmt.Sprintf("%s/%s", h.upstreamURL, url.PathEscape(packageName)) - - body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, upstreamURL, contentTypeJSON) + body, err := metadata() if err != nil { h.proxy.Logger.Warn("cooldown: could not fetch npm metadata for download check", "package", packageName, "version", version, "error", err) return false } - var metadata struct { + var document struct { Time map[string]string `json:"time"` } - if err := json.Unmarshal(body, &metadata); err != nil { + if err := json.Unmarshal(body, &document); err != nil { h.proxy.Logger.Warn("cooldown: could not parse npm metadata for download check", "package", packageName, "version", version, "error", err) return false } - published, ok := metadata.Time[version] + published, ok := document.Time[version] if !ok { return false } diff --git a/internal/handler/npm_tarball_test.go b/internal/handler/npm_tarball_test.go new file mode 100644 index 00000000..303f2184 --- /dev/null +++ b/internal/handler/npm_tarball_test.go @@ -0,0 +1,154 @@ +package handler + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/cooldown" + "github.com/git-pkgs/registries/fetch" +) + +func TestNPMContentAddressedTarball(t *testing.T) { + for _, tc := range []struct { + cacheMetadata, stream bool + metadata, tarballs int64 + }{ + {false, false, 2, 1}, + {false, true, 3, 2}, + {true, false, 1, 1}, + {true, true, 1, 2}, + } { + t.Run(fmt.Sprintf("metadata=%t/stream=%t", tc.cacheMetadata, tc.stream), func(t *testing.T) { + var metadataCalls, tarballCalls atomic.Int64 + upstream := npmContentAddressedRegistry(t, &metadataCalls, &tarballCalls) + p, _, _, _ := setupTestProxy(t) + p.CacheMetadata = tc.cacheMetadata + p.MetadataTTL = time.Hour + p.StreamArtifacts = tc.stream + p.Cooldown = &cooldown.Config{Default: "7d"} + p.HTTPClient = upstream.Client() + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0)) + p.Fetcher = fetcher + t.Cleanup(func() { _ = fetcher.Close() }) + h := NewNPMHandler(p, "http://proxy.test", upstream.URL+"/registry") + routes := http.StripPrefix("/npm", h.Routes()) + metadata := httptest.NewRecorder() + routes.ServeHTTP(metadata, httptest.NewRequest(http.MethodGet, "/npm/@example/widget", nil)) + if metadata.Code != http.StatusOK { + t.Fatalf("metadata status = %d: %s", metadata.Code, metadata.Body.String()) + } + tarball := npmVersionTarball(metadata.Body.Bytes(), testVersion100) + if want := "http://proxy.test/npm/@example%2Fwidget/-/widget-1.0.0.tgz"; tarball != want { + t.Fatalf("rewritten tarball = %q, want %q", tarball, want) + } + for i := range 2 { + response := httptest.NewRecorder() + routes.ServeHTTP(response, httptest.NewRequest(http.MethodGet, tarball, nil)) + if response.Code != http.StatusOK || response.Body.String() != "tarball bytes" { + t.Fatalf("download %d = %d %q", i, response.Code, response.Body.String()) + } + if !tc.stream { + upstream.Close() + } + } + if metadataCalls.Load() != tc.metadata || tarballCalls.Load() != tc.tarballs { + t.Errorf("upstream requests: metadata=%d tarballs=%d, want %d %d", metadataCalls.Load(), tarballCalls.Load(), tc.metadata, tc.tarballs) + } + }) + } +} + +func npmContentAddressedRegistry(t *testing.T, metadataCalls, tarballCalls *atomic.Int64) *httptest.Server { + t.Helper() + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/registry/@example/widget": + metadataCalls.Add(1) + w.Header().Set("Content-Type", contentTypeJSON) + _, _ = fmt.Fprintf(w, `{"name":"@example/widget","time":{"1.0.0":%q},"versions":{"1.0.0":{"dist":{"tarball":%q}}}}`, + time.Now().Add(-30*24*time.Hour).Format(time.RFC3339), upstream.URL+"/registry/download/@example/widget/1.0.0/abc123?token=example") + case "/registry/download/@example/widget/1.0.0/abc123": + if r.URL.RawQuery != "token=example" { + t.Errorf("tarball query = %q", r.URL.RawQuery) + } + tarballCalls.Add(1) + _, _ = io.WriteString(w, "tarball bytes") + default: + t.Errorf("unexpected upstream request: %s", r.URL) + http.NotFound(w, r) + } + })) + t.Cleanup(upstream.Close) + return upstream +} + +func TestNPMTarballMetadataFallback(t *testing.T) { + for _, body := range []string{`{"versions":{}}`, `{"versions":{"1.0.0":{"dist":{}}}}`, `not json`, "unavailable"} { + t.Run(body, func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if body == "unavailable" { + w.WriteHeader(http.StatusServiceUnavailable) + } + _, _ = io.WriteString(w, body) + })) + t.Cleanup(upstream.Close) + p, _, _, fetcher := setupTestProxy(t) + p.CacheMetadata = true + p.MetadataTTL = time.Hour + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("package"))} + h := NewNPMHandler(p, "http://proxy.test", upstream.URL) + if body != "unavailable" { + if _, _, err := p.FetchOrCacheMetadata(t.Context(), "npm", "@example/widget", upstream.URL); err != nil { + t.Fatal(err) + } + upstream.Close() + } + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/@example/widget/-/widget-1.0.0.tgz", nil)) + if response.Code != http.StatusOK { + t.Fatalf("status = %d: %s", response.Code, response.Body.String()) + } + if want := upstream.URL + "/@example/widget/-/widget-1.0.0.tgz"; fetcher.fetchedURL != want { + t.Errorf("fetched URL = %q, want %q", fetcher.fetchedURL, want) + } + }) + } +} + +func TestNPMTarballRejectsUnsafeMetadataURL(t *testing.T) { + for _, target := range []string{ + "https://outside.invalid/package.tgz", "//outside.invalid/package.tgz", "/registry/package.tgz", + "UPSTREAM/outside/package.tgz", "UPSTREAM/registry-other/package.tgz", + "UPSTREAM/registry/../outside/package.tgz", "UPSTREAM/registry/%2e%2e/outside/package.tgz", + "UPSTREAM/registry/%252e%252e/outside/package.tgz", "UPSTREAM/registry/..%2foutside/package.tgz", + "UPSTREAM/registry/..%5coutside/package.tgz", "UPSTREAM/registry/package.tgz#fragment", + "USERINFO/registry/package.tgz", "SCHEME/registry/package.tgz", + } { + t.Run(target, func(t *testing.T) { + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + address, _ := url.Parse(upstream.URL) + address.User = url.UserPassword("user", "secret") + tarball := strings.NewReplacer("UPSTREAM", upstream.URL, "USERINFO", address.String(), "SCHEME", strings.Replace(upstream.URL, "http:", "https:", 1)).Replace(target) + _ = json.NewEncoder(w).Encode(map[string]any{"versions": map[string]any{testVersion100: map[string]any{"dist": map[string]string{"tarball": tarball}}}}) + })) + t.Cleanup(upstream.Close) + p, _, _, fetcher := setupTestProxy(t) + h := NewNPMHandler(p, "http://proxy.test", upstream.URL+"/registry") + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/widget/-/widget-1.0.0.tgz", nil)) + if response.Code != http.StatusBadGateway || fetcher.fetchCalled { + t.Errorf("status = %d, fetched = %t; body: %s", response.Code, fetcher.fetchCalled, response.Body.String()) + } + }) + } +} diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index d0c38890..5d8378fb 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -538,22 +538,13 @@ func TestNPMDownloadCooldown(t *testing.T) { } func TestNPMDownloadCooldownDisabled(t *testing.T) { - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - t.Error("metadata must not be fetched when cooldown is disabled") - w.WriteHeader(http.StatusInternalServerError) - })) - defer upstream.Close() - - proxy, _, _, fetcher := setupTestProxy(t) - proxy.HTTPClient = upstream.Client() - fetcher.artifact = &fetch.Artifact{ - Body: io.NopCloser(strings.NewReader("tarball data")), - ContentType: "application/octet-stream", - } - - h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + proxy, _, _, _ := setupTestProxy(t) + h := NewNPMHandler(proxy, "http://proxy.test", "") - if h.versionInCooldown(httptest.NewRequest(http.MethodGet, "/", nil), "leftpad", testVersion100) { + if h.versionInCooldown("leftpad", testVersion100, func() ([]byte, error) { + t.Fatal("cooldown must not request metadata when disabled") + return nil, nil + }) { t.Error("versionInCooldown = true, want false when cooldown is not configured") } } @@ -577,13 +568,10 @@ func TestNPMDownloadCooldownUsesStoredPublishTime(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - proxy, db, _, fetcher := setupTestProxy(t) + proxy, db, store, _ := setupTestProxy(t) proxy.HTTPClient = upstream.Client() proxy.Cooldown = &cooldown.Config{Default: "7d"} - fetcher.artifact = &fetch.Artifact{ - Body: io.NopCloser(strings.NewReader("tarball data")), - ContentType: "application/octet-stream", - } + seedPackage(t, db, store, "npm", "leftpad", tt.version, "leftpad-"+tt.version+".tgz", "tarball data") if err := db.SetVersionPublishedAt("pkg:npm/leftpad@"+tt.version, "pkg:npm/leftpad", tt.publishedAt); err != nil { t.Fatalf("seeding publish time failed: %v", err)