From 09b9ef98f0b4b812232ec980e148f7bf0e32bb26 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Thu, 8 Oct 2026 13:06:48 +0200 Subject: [PATCH 1/5] npm: version lookup returns 502 Looking up a specific version will trigger a 502 instead of return the version metadata. --- internal/handler/npm.go | 66 ++++++++++++++-- internal/handler/npm_test.go | 141 +++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 6 deletions(-) diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 31c53419..70601ea7 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -158,15 +158,28 @@ func (h *NPMHandler) handleSecurity(w http.ResponseWriter, r *http.Request) { // handlePackageMetadata proxies package metadata from upstream and rewrites tarball URLs. func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Request) { - packageName := h.extractPackageName(r) + packagePath := h.extractPackageName(r) + if packagePath == "" || containsPathTraversal(packagePath) { + JSONError(w, http.StatusBadRequest, "invalid package name") + return + } + // A scoped name is always @scope/pkg; the registry has no GET for @scope alone. + if strings.HasPrefix(packagePath, "@") && !strings.Contains(packagePath, "/") { + JSONError(w, http.StatusBadRequest, "invalid package name") + return + } + packageName, registryPath := npmMetadataPath(packagePath) if packageName == "" { JSONError(w, http.StatusBadRequest, "invalid package name") return } - h.proxy.Logger.Info("npm metadata request", "package", packageName) + h.proxy.Logger.Info("npm metadata request", "package", packageName, "path", packagePath) - upstreamURL := fmt.Sprintf("%s/%s", h.upstreamURL, url.PathEscape(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. + upstreamURL := h.upstreamURL + "/" + registryPath // Prefer the smaller abbreviated packument format but include application/json // as a fallback so upstreams that reject the abbreviated type (e.g. JFrog @@ -180,14 +193,14 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques accept = contentTypeJSON } - if rewritten, ok := h.proxy.storedRewrite("npm", packageName, h.proxyURL, packageName); ok { + if rewritten, ok := h.proxy.storedRewrite("npm", packagePath, h.proxyURL, packagePath); ok { w.Header().Set(headerContentType, contentTypeJSON) w.WriteHeader(http.StatusOK) _, _ = w.Write(rewritten) return } - body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, upstreamURL, accept) + body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packagePath, upstreamURL, accept) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { JSONError(w, http.StatusNotFound, "package not found") @@ -198,13 +211,17 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques return } - rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, packageName, body, func(b []byte) ([]byte, error) { + rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, packagePath, body, func(b []byte) ([]byte, error) { return h.rewriteMetadata(packageName, b) }) if err != nil { if r.Context().Err() != nil { return // the client left while waiting on a shared rewrite } + if errors.Is(err, ErrVersionDenied) { + JSONError(w, http.StatusForbidden, err.Error()) + return + } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { JSONError(w, http.StatusBadGateway, "failed to filter package metadata") return @@ -240,6 +257,18 @@ func (h *NPMHandler) rewriteMetadata(packageName string, body []byte) ([]byte, e versionsRaw := doc.value(doc.versionsAt) if len(versionsRaw) == 0 || versionsRaw[0] != '{' { + version, ok, err := lookupJSONString(body, "version") + if err != nil { + return nil, err + } + if ok { + if h.proxy.versionDenied("npm", packageName, version) { + return nil, ErrVersionDenied + } + var out bytes.Buffer + h.writeNPMVersion(&out, packageName, version, body) + return out.Bytes(), nil + } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { return nil, errors.New("npm metadata has no versions object") } @@ -713,6 +742,31 @@ func (h *NPMHandler) versionInCooldown(packageName, version string, metadata fun return !h.proxy.Cooldown.IsAllowed("npm", canonicalPackagePURL("npm", packageName), publishedAt) } +// npmMetadataPath returns the package name and the encoded registry path. +// Examples: lodash → lodash; lodash/4.17.21 → lodash/4.17.21; +// @babel/core → @babel%2Fcore; @babel/core/7.23.0 → @babel%2Fcore/7.23.0. +func npmMetadataPath(packagePath string) (packageName, registryPath string) { + // GET /npm/ has no package segment; handlePackageMetadata rejects that earlier. + if packagePath == "" { + return "", "" + } + if !strings.Contains(packagePath, "/") { + return packagePath, url.PathEscape(packagePath) + } + if strings.HasPrefix(packagePath, "@") { + scope, rest, _ := strings.Cut(packagePath, "/") + name, version, ok := strings.Cut(rest, "/") + packageName = scope + "/" + name + registryPath = url.PathEscape(scope) + "%2F" + url.PathEscape(name) + if ok { + registryPath += "/" + url.PathEscape(version) + } + return packageName, registryPath + } + name, version, _ := strings.Cut(packagePath, "/") + return name, url.PathEscape(name) + "/" + url.PathEscape(version) +} + func escapeNPMDownloadPackage(packageName string) string { scope, name, scoped := strings.Cut(packageName, "/") if scoped && strings.HasPrefix(scope, "@") && len(scope) > 1 && name != "" && !strings.Contains(name, "/") { diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index 5d8378fb..aed04434 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1047,3 +1047,144 @@ func TestNPMTarballStillRoutesToDownload(t *testing.T) { t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusOK, w.Body.String()) } } + +func TestNPMMetadataPath(t *testing.T) { + tests := []struct { + path, wantName, wantRegistry string + }{ + {"", "", ""}, + {"lodash", "lodash", "lodash"}, + {"lodash/4.17.21", "lodash", "lodash/4.17.21"}, + {"lodash/latest", "lodash", "lodash/latest"}, + {"@babel/core", "@babel/core", "@babel%2Fcore"}, + {"@babel/core/7.23.0", "@babel/core", "@babel%2Fcore/7.23.0"}, + } + for _, tt := range tests { + name, registry := npmMetadataPath(tt.path) + if name != tt.wantName || registry != tt.wantRegistry { + t.Errorf("npmMetadataPath(%q) = (%q, %q), want (%q, %q)", + tt.path, name, registry, tt.wantName, tt.wantRegistry) + } + } +} + +func TestNPMVersionMetadataKeepsVersionSegment(t *testing.T) { + tests := []struct { + request, upstreamPath, wantTarball string + body string + }{ + { + request: "/lodash/4.17.21", + upstreamPath: "/lodash/4.17.21", + wantTarball: "http://proxy.local/npm/lodash/-/lodash-4.17.21.tgz", + body: `{ + "name": "lodash", + "version": "4.17.21", + "dist": {"tarball": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz"} + }`, + }, + { + request: "/@babel/core/7.23.0", + upstreamPath: "/@babel%2Fcore/7.23.0", + wantTarball: "http://proxy.local/npm/@babel%2Fcore/-/core-7.23.0.tgz", + body: `{ + "name": "@babel/core", + "version": "7.23.0", + "dist": {"tarball": "https://registry.npmjs.org/@babel/core/-/core-7.23.0.tgz"} + }`, + }, + } + for _, tt := range tests { + t.Run(tt.request, func(t *testing.T) { + var gotPath string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + if gotPath != tt.upstreamPath { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, tt.body) + })) + defer upstream.Close() + + h := &NPMHandler{proxy: testProxy(), upstreamURL: upstream.URL, proxyURL: "http://proxy.local"} + w := httptest.NewRecorder() + h.handlePackageMetadata(w, httptest.NewRequest(http.MethodGet, tt.request, nil)) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + if gotPath != tt.upstreamPath { + t.Fatalf("upstream path = %q, want %q", gotPath, tt.upstreamPath) + } + var result map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("parse response: %v", err) + } + if result["dist"].(map[string]any)["tarball"] != tt.wantTarball { + t.Errorf("tarball = %v", result["dist"].(map[string]any)["tarball"]) + } + }) + } +} + +func TestNPMMetadataRejectsEmptyPath(t *testing.T) { + h := NewNPMHandler(testProxy(), "http://proxy.local", "http://unused.test") + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/", nil)) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String()) + } +} + +func TestNPMMetadataRejectsScopeOnly(t *testing.T) { + h := &NPMHandler{proxy: testProxy(), upstreamURL: "http://unused.test", proxyURL: "http://proxy.local"} + w := httptest.NewRecorder() + h.handlePackageMetadata(w, httptest.NewRequest(http.MethodGet, "/@babel", nil)) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String()) + } +} + +func TestNPMVersionMetadataRejectsTraversal(t *testing.T) { + called := false + upstream := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + called = true + })) + defer upstream.Close() + + h := &NPMHandler{proxy: testProxy(), upstreamURL: upstream.URL, proxyURL: "http://proxy.local"} + req := httptest.NewRequest(http.MethodGet, "/lodash/x", nil) + req.URL.Path = "/lodash/.." + w := httptest.NewRecorder() + h.handlePackageMetadata(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String()) + } + if called { + t.Fatal("upstream contacted for traversal path") + } +} + +func TestNPMVersionMetadataDenied(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, `{ + "name": "lodash", + "version": "4.17.21", + "dist": {"tarball": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz"} + }`) + })) + defer upstream.Close() + + proxy := testProxy() + setTestDenylist(t, proxy, "pkg:npm/lodash@4.17.21") + h := &NPMHandler{proxy: proxy, upstreamURL: upstream.URL, proxyURL: "http://proxy.local"} + + w := httptest.NewRecorder() + h.handlePackageMetadata(w, httptest.NewRequest(http.MethodGet, "/lodash/4.17.21", nil)) + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body: %s", w.Code, w.Body.String()) + } +} From 8e25380047dd13b90143f60b4d14573ea2c899d2 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Thu, 8 Oct 2026 13:12:10 +0200 Subject: [PATCH 2/5] scope only not handled explicitly --- internal/handler/npm.go | 5 ----- internal/handler/npm_test.go | 9 --------- 2 files changed, 14 deletions(-) diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 70601ea7..1fdb966f 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -163,11 +163,6 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques JSONError(w, http.StatusBadRequest, "invalid package name") return } - // A scoped name is always @scope/pkg; the registry has no GET for @scope alone. - if strings.HasPrefix(packagePath, "@") && !strings.Contains(packagePath, "/") { - JSONError(w, http.StatusBadRequest, "invalid package name") - return - } packageName, registryPath := npmMetadataPath(packagePath) if packageName == "" { JSONError(w, http.StatusBadRequest, "invalid package name") diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index aed04434..37f17146 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1138,15 +1138,6 @@ func TestNPMMetadataRejectsEmptyPath(t *testing.T) { } } -func TestNPMMetadataRejectsScopeOnly(t *testing.T) { - h := &NPMHandler{proxy: testProxy(), upstreamURL: "http://unused.test", proxyURL: "http://proxy.local"} - w := httptest.NewRecorder() - h.handlePackageMetadata(w, httptest.NewRequest(http.MethodGet, "/@babel", nil)) - if w.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String()) - } -} - func TestNPMVersionMetadataRejectsTraversal(t *testing.T) { called := false upstream := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { From 35a720e4701a33e3cf9bfd80ee237725b6040299 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Mon, 12 Oct 2026 14:08:26 +0200 Subject: [PATCH 3/5] honor cooldown in version lookups tested (set default cooldown to 7d and 8.0.7 is released four days ago or something): ``` $ curl -sS 'http://localhost:8080/npm/@babel%2Fcore/8.0.7' | jq { "error": "package not found" } ``` --- internal/handler/npm.go | 106 ++++++++++++++++++++++++++++ internal/handler/npm_test.go | 129 +++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+) diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 1fdb966f..f922c75f 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -195,6 +195,12 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques return } + specifier, versionLookup := npmVersionSpecifier(packagePath, packageName) + if versionLookup && h.npmVersionMetadataUsesFilteredPackument(packageName) { + h.handleFilteredNPMVersionMetadata(w, r, packageName, packagePath, specifier, accept) + return + } + body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packagePath, upstreamURL, accept) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { @@ -217,6 +223,10 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques JSONError(w, http.StatusForbidden, err.Error()) return } + if errors.Is(err, ErrUpstreamNotFound) { + JSONError(w, http.StatusNotFound, "package not found") + return + } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { JSONError(w, http.StatusBadGateway, "failed to filter package metadata") return @@ -234,6 +244,102 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques _, _ = w.Write(rewritten) } +// handleFilteredNPMVersionMetadata serves /pkg/ and /pkg/ from the +// filtered packument so cooldown and dist-tags match the root metadata response. +func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r *http.Request, packageName, packagePath, specifier, accept string) { + _, packumentPath := npmMetadataPath(packageName) + packument, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", packageName, h.upstreamURL+"/"+packumentPath, accept) + if err != nil { + if errors.Is(err, ErrUpstreamNotFound) { + JSONError(w, http.StatusNotFound, "package not found") + return + } + h.proxy.Logger.Error("failed to fetch npm metadata", "error", err) + JSONError(w, http.StatusBadGateway, "failed to fetch from upstream") + return + } + + rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, packagePath, packument, func(b []byte) ([]byte, error) { + return h.rewriteNPMVersionFromPackument(packageName, specifier, b) + }) + if err != nil { + if r.Context().Err() != nil { + return + } + if errors.Is(err, ErrVersionDenied) { + JSONError(w, http.StatusForbidden, err.Error()) + return + } + if errors.Is(err, ErrUpstreamNotFound) { + JSONError(w, http.StatusNotFound, "package not found") + return + } + if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { + JSONError(w, http.StatusBadGateway, "failed to filter package metadata") + return + } + h.proxy.Logger.Warn("failed to rewrite npm version metadata", "error", err) + JSONError(w, http.StatusBadGateway, "failed to filter package metadata") + return + } + + w.Header().Set(headerContentType, contentTypeJSON) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(rewritten) +} + +// npmVersionSpecifier returns the version or dist-tag segment after the package +// name in a metadata path, for example "4.17.21" or "latest". +func npmVersionSpecifier(packagePath, packageName string) (string, bool) { + if packagePath == packageName { + return "", false + } + specifier, ok := strings.CutPrefix(packagePath, packageName+"/") + return specifier, ok +} + +func (h *NPMHandler) npmVersionMetadataUsesFilteredPackument(packageName string) bool { + if h.proxy.Cooldown != nil && h.proxy.Cooldown.Enabled() { + return true + } + if h.proxy.Denylist != nil { + return len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 + } + return false +} + +// rewriteNPMVersionFromPackument picks one version entry out of an already +// filtered packument and rewrites its tarball URL. +func (h *NPMHandler) rewriteNPMVersionFromPackument(packageName, specifier string, packument []byte) ([]byte, error) { + filtered, err := h.rewriteMetadata(packageName, packument) + if err != nil { + return nil, err + } + + target := specifier + if resolved, ok, err := lookupJSONString(filtered, "dist-tags", specifier); err != nil { + return nil, err + } else if ok { + target = resolved + } + + if h.proxy.versionDenied("npm", packageName, target) { + return nil, ErrVersionDenied + } + + versionRaw, err := lookupJSON(filtered, "versions", target) + if err != nil { + return nil, err + } + if len(versionRaw) == 0 { + return nil, ErrUpstreamNotFound + } + + var out bytes.Buffer + h.writeNPMVersion(&out, packageName, target, versionRaw) + return out.Bytes(), nil +} + // rewriteMetadata rewrites tarball URLs in npm package metadata to point at this proxy. // If cooldown is enabled, versions published too recently are filtered out. // diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index 37f17146..87a011b4 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1159,6 +1159,135 @@ func TestNPMVersionMetadataRejectsTraversal(t *testing.T) { } } +func TestNPMVersionMetadataCooldown(t *testing.T) { + now := time.Now() + packument := `{ + "name": "demo", + "dist-tags": {"latest": "2.0.0"}, + "time": { + "1.0.0": "` + now.Add(-30*24*time.Hour).Format(time.RFC3339) + `", + "2.0.0": "` + now.Add(-1*time.Hour).Format(time.RFC3339) + `" + }, + "versions": { + "1.0.0": { + "name": "demo", + "version": "1.0.0", + "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-1.0.0.tgz"} + }, + "2.0.0": { + "name": "demo", + "version": "2.0.0", + "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-2.0.0.tgz"} + } + } + }` + version200 := `{ + "name": "demo", + "version": "2.0.0", + "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-2.0.0.tgz"} + }` + version100 := `{ + "name": "demo", + "version": "1.0.0", + "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-1.0.0.tgz"} + }` + + var upstreamPaths []string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamPaths = append(upstreamPaths, r.URL.Path) + w.Header().Set("Content-Type", contentTypeJSON) + switch r.URL.Path { + case "/demo": + _, _ = io.WriteString(w, packument) + case "/demo/latest", "/demo/2.0.0": + _, _ = io.WriteString(w, version200) + case "/demo/1.0.0": + _, _ = io.WriteString(w, version100) + default: + http.NotFound(w, r) + } + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.Cooldown = &cooldown.Config{Default: "7d"} + + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + srv := httptest.NewServer(h.Routes()) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/demo") + if err != nil { + t.Fatalf("packument request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + t.Fatalf("packument status = %d, want 200", resp.StatusCode) + } + var packumentResult map[string]any + if err := json.NewDecoder(resp.Body).Decode(&packumentResult); err != nil { + t.Fatalf("decode packument: %v", err) + } + distTags := packumentResult["dist-tags"].(map[string]any) + if distTags["latest"] != testVersion100 { + t.Fatalf("dist-tags.latest = %v, want %s", distTags["latest"], testVersion100) + } + if _, ok := packumentResult["versions"].(map[string]any)["2.0.0"]; ok { + t.Fatal("packument should not include version 2.0.0") + } + + tests := []struct { + path string + wantStatus int + wantVersion string + }{ + {"/demo/latest", http.StatusOK, testVersion100}, + {"/demo/1.0.0", http.StatusOK, testVersion100}, + {"/demo/2.0.0", http.StatusNotFound, ""}, + } + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + before := len(upstreamPaths) + resp, err := http.Get(srv.URL + tt.path) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != tt.wantStatus { + t.Fatalf("status = %d, want %d; body: %s", resp.StatusCode, tt.wantStatus, readNPMTestBody(t, resp.Body)) + } + for _, path := range upstreamPaths[before:] { + if path == tt.path { + t.Fatalf("upstream must not be called for version path %s when answering from the filtered packument", path) + } + } + if tt.wantVersion == "" { + return + } + var result map[string]any + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + t.Fatalf("decode response: %v", err) + } + if result["version"] != tt.wantVersion { + t.Fatalf("version = %v, want %s", result["version"], tt.wantVersion) + } + wantTarball := "http://proxy.test/npm/demo/-/demo-" + tt.wantVersion + ".tgz" + if result["dist"].(map[string]any)["tarball"] != wantTarball { + t.Fatalf("tarball = %v, want %s", result["dist"].(map[string]any)["tarball"], wantTarball) + } + }) + } +} + +func readNPMTestBody(t *testing.T, r io.Reader) string { + b, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read body: %v", err) + } + return string(b) +} + func TestNPMVersionMetadataDenied(t *testing.T) { upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, `{ From ca0a442497cc9a776fb0340615b167579feca8db Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Mon, 12 Oct 2026 14:24:10 +0200 Subject: [PATCH 4/5] package not found -> version not found --- internal/handler/handler.go | 8 ++++++++ internal/handler/npm.go | 22 +++++++++++++--------- internal/handler/npm_test.go | 13 ++++++++++--- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 449fcef7..036e2a5f 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1030,6 +1030,14 @@ var ErrArtifactBlocked = errors.New("artifact blocked by security scan") // ErrVersionDenied indicates an operator explicitly denied this package version. var ErrVersionDenied = errors.New("package version blocked by denylist") +// ErrVersionNotFound indicates the requested version is not available through +// the proxy, including when cooldown or other filtering removed it. +var ErrVersionNotFound = errors.New("version not found") + +// ErrPackageNotFound indicates the requested package metadata is not available +// through the proxy or upstream registry. +var ErrPackageNotFound = errors.New("package not found") + // serveArtifactError writes response for a failed fetch: // 404 when upstream reports artifact missing, 403 when a security scan or // denylist blocked the artifact, 502 otherwise. diff --git a/internal/handler/npm.go b/internal/handler/npm.go index e41a9ee1..e2c61890 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -223,7 +223,11 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", metadataCacheKey, upstreamURL, accept) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { - JSONError(w, http.StatusNotFound, "package not found") + if versionLookup { + JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) + } else { + JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error()) + } return } h.proxy.Logger.Error("failed to fetch npm metadata", "error", err) @@ -242,8 +246,8 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques JSONError(w, http.StatusForbidden, err.Error()) return } - if errors.Is(err, ErrUpstreamNotFound) { - JSONError(w, http.StatusNotFound, "package not found") + if errors.Is(err, ErrVersionNotFound) { + JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) return } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { @@ -270,7 +274,7 @@ func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r * packument, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", cacheKey, registryURL+"/"+packumentPath, accept) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { - JSONError(w, http.StatusNotFound, "package not found") + JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error()) return } h.proxy.Logger.Error("failed to fetch npm metadata", "error", err) @@ -289,8 +293,8 @@ func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r * JSONError(w, http.StatusForbidden, err.Error()) return } - if errors.Is(err, ErrUpstreamNotFound) { - JSONError(w, http.StatusNotFound, "package not found") + if errors.Is(err, ErrVersionNotFound) { + JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) return } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { @@ -361,7 +365,7 @@ func (h *NPMHandler) rewriteNPMVersionFromPackument(packageName, specifier strin return nil, err } if len(versionRaw) == 0 { - return nil, ErrUpstreamNotFound + return nil, ErrVersionNotFound } var out bytes.Buffer @@ -753,7 +757,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { 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") + JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) return } @@ -761,7 +765,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { if err != nil { switch { case errors.Is(err, ErrUpstreamNotFound): - JSONError(w, http.StatusNotFound, "package not found") + JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error()) case errors.Is(err, ErrArtifactBlocked), errors.Is(err, ErrVersionDenied): JSONError(w, http.StatusForbidden, err.Error()) default: diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index 87a011b4..86fde9a5 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1241,10 +1241,11 @@ func TestNPMVersionMetadataCooldown(t *testing.T) { path string wantStatus int wantVersion string + wantError string }{ - {"/demo/latest", http.StatusOK, testVersion100}, - {"/demo/1.0.0", http.StatusOK, testVersion100}, - {"/demo/2.0.0", http.StatusNotFound, ""}, + {"/demo/latest", http.StatusOK, testVersion100, ""}, + {"/demo/1.0.0", http.StatusOK, testVersion100, ""}, + {"/demo/2.0.0", http.StatusNotFound, "", "version not found"}, } for _, tt := range tests { t.Run(tt.path, func(t *testing.T) { @@ -1262,6 +1263,12 @@ func TestNPMVersionMetadataCooldown(t *testing.T) { t.Fatalf("upstream must not be called for version path %s when answering from the filtered packument", path) } } + if tt.wantError != "" { + if !strings.Contains(readNPMTestBody(t, resp.Body), tt.wantError) { + t.Fatalf("error body should mention %q", tt.wantError) + } + return + } if tt.wantVersion == "" { return } From 1a70c962beda2e20c46b364f24999bfac988189c Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Mon, 12 Oct 2026 14:40:08 +0200 Subject: [PATCH 5/5] gocognit --- internal/handler/npm_test.go | 179 +++++++++++++++++------------------ 1 file changed, 89 insertions(+), 90 deletions(-) diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index 86fde9a5..5f45feb8 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1160,14 +1160,91 @@ func TestNPMVersionMetadataRejectsTraversal(t *testing.T) { } func TestNPMVersionMetadataCooldown(t *testing.T) { + proxyURL, upstreamPaths := npmCooldownMetadataHarness(t) + + resp, err := http.Get(proxyURL + "/demo") + if err != nil { + t.Fatalf("packument request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + t.Fatalf("packument status = %d, want 200", resp.StatusCode) + } + var packument map[string]any + if err := json.NewDecoder(resp.Body).Decode(&packument); err != nil { + t.Fatalf("decode packument: %v", err) + } + distTags := packument["dist-tags"].(map[string]any) + if distTags["latest"] != testVersion100 { + t.Fatalf("dist-tags.latest = %v, want %s", distTags["latest"], testVersion100) + } + if _, ok := packument["versions"].(map[string]any)["2.0.0"]; ok { + t.Fatal("packument should not include version 2.0.0") + } + + for _, tt := range []struct { + path string + wantStatus int + wantVersion string + wantError string + }{ + {"/demo/latest", http.StatusOK, testVersion100, ""}, + {"/demo/1.0.0", http.StatusOK, testVersion100, ""}, + {"/demo/2.0.0", http.StatusNotFound, "", "version not found"}, + } { + t.Run(tt.path, func(t *testing.T) { + testNPMVersionMetadataCooldownPath(t, proxyURL, upstreamPaths, tt.path, tt.wantStatus, tt.wantVersion, tt.wantError) + }) + } +} + +// testNPMVersionMetadataCooldownPath is a subtest helper so the parent test stays +// under gocognit's complexity limit. +func testNPMVersionMetadataCooldownPath(t *testing.T, proxyURL string, upstreamPaths *[]string, path string, wantStatus int, wantVersion, wantError string) { + t.Helper() + before := len(*upstreamPaths) + resp, err := http.Get(proxyURL + path) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + body := readNPMTestBody(t, resp.Body) + if resp.StatusCode != wantStatus { + t.Fatalf("status = %d, want %d; body: %s", resp.StatusCode, wantStatus, body) + } + for _, upstreamPath := range (*upstreamPaths)[before:] { + if upstreamPath == path { + t.Fatalf("upstream must not be called for version path %s when answering from the filtered packument", path) + } + } + if wantError != "" { + if !strings.Contains(body, wantError) { + t.Fatalf("error body should mention %q, got %s", wantError, body) + } + return + } + var versionDoc map[string]any + if err := json.Unmarshal([]byte(body), &versionDoc); err != nil { + t.Fatalf("decode response: %v", err) + } + if versionDoc["version"] != wantVersion { + t.Fatalf("version = %v, want %s", versionDoc["version"], wantVersion) + } + wantTarball := "http://proxy.test/npm/demo/-/demo-" + wantVersion + ".tgz" + if versionDoc["dist"].(map[string]any)["tarball"] != wantTarball { + t.Fatalf("tarball = %v, want %s", versionDoc["dist"].(map[string]any)["tarball"], wantTarball) + } +} + +func npmCooldownMetadataHarness(t *testing.T) (proxyURL string, upstreamPaths *[]string) { + t.Helper() now := time.Now() + old := now.Add(-30 * 24 * time.Hour).Format(time.RFC3339) + recent := now.Add(-1 * time.Hour).Format(time.RFC3339) packument := `{ "name": "demo", "dist-tags": {"latest": "2.0.0"}, - "time": { - "1.0.0": "` + now.Add(-30*24*time.Hour).Format(time.RFC3339) + `", - "2.0.0": "` + now.Add(-1*time.Hour).Format(time.RFC3339) + `" - }, + "time": {"1.0.0": "` + old + `", "2.0.0": "` + recent + `"}, "versions": { "1.0.0": { "name": "demo", @@ -1181,20 +1258,12 @@ func TestNPMVersionMetadataCooldown(t *testing.T) { } } }` - version200 := `{ - "name": "demo", - "version": "2.0.0", - "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-2.0.0.tgz"} - }` - version100 := `{ - "name": "demo", - "version": "1.0.0", - "dist": {"tarball": "https://registry.npmjs.org/demo/-/demo-1.0.0.tgz"} - }` + version200 := `{"name":"demo","version":"2.0.0","dist":{"tarball":"https://registry.npmjs.org/demo/-/demo-2.0.0.tgz"}}` + version100 := `{"name":"demo","version":"1.0.0","dist":{"tarball":"https://registry.npmjs.org/demo/-/demo-1.0.0.tgz"}}` - var upstreamPaths []string + var paths []string upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - upstreamPaths = append(upstreamPaths, r.URL.Path) + paths = append(paths, r.URL.Path) w.Header().Set("Content-Type", contentTypeJSON) switch r.URL.Path { case "/demo": @@ -1207,84 +1276,14 @@ func TestNPMVersionMetadataCooldown(t *testing.T) { http.NotFound(w, r) } })) - defer upstream.Close() + t.Cleanup(upstream.Close) proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = upstream.Client() proxy.Cooldown = &cooldown.Config{Default: "7d"} - - h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) - srv := httptest.NewServer(h.Routes()) - defer srv.Close() - - resp, err := http.Get(srv.URL + "/demo") - if err != nil { - t.Fatalf("packument request failed: %v", err) - } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusOK { - t.Fatalf("packument status = %d, want 200", resp.StatusCode) - } - var packumentResult map[string]any - if err := json.NewDecoder(resp.Body).Decode(&packumentResult); err != nil { - t.Fatalf("decode packument: %v", err) - } - distTags := packumentResult["dist-tags"].(map[string]any) - if distTags["latest"] != testVersion100 { - t.Fatalf("dist-tags.latest = %v, want %s", distTags["latest"], testVersion100) - } - if _, ok := packumentResult["versions"].(map[string]any)["2.0.0"]; ok { - t.Fatal("packument should not include version 2.0.0") - } - - tests := []struct { - path string - wantStatus int - wantVersion string - wantError string - }{ - {"/demo/latest", http.StatusOK, testVersion100, ""}, - {"/demo/1.0.0", http.StatusOK, testVersion100, ""}, - {"/demo/2.0.0", http.StatusNotFound, "", "version not found"}, - } - for _, tt := range tests { - t.Run(tt.path, func(t *testing.T) { - before := len(upstreamPaths) - resp, err := http.Get(srv.URL + tt.path) - if err != nil { - t.Fatalf("request failed: %v", err) - } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != tt.wantStatus { - t.Fatalf("status = %d, want %d; body: %s", resp.StatusCode, tt.wantStatus, readNPMTestBody(t, resp.Body)) - } - for _, path := range upstreamPaths[before:] { - if path == tt.path { - t.Fatalf("upstream must not be called for version path %s when answering from the filtered packument", path) - } - } - if tt.wantError != "" { - if !strings.Contains(readNPMTestBody(t, resp.Body), tt.wantError) { - t.Fatalf("error body should mention %q", tt.wantError) - } - return - } - if tt.wantVersion == "" { - return - } - var result map[string]any - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - t.Fatalf("decode response: %v", err) - } - if result["version"] != tt.wantVersion { - t.Fatalf("version = %v, want %s", result["version"], tt.wantVersion) - } - wantTarball := "http://proxy.test/npm/demo/-/demo-" + tt.wantVersion + ".tgz" - if result["dist"].(map[string]any)["tarball"] != wantTarball { - t.Fatalf("tarball = %v, want %s", result["dist"].(map[string]any)["tarball"], wantTarball) - } - }) - } + srv := httptest.NewServer(NewNPMHandler(proxy, "http://proxy.test", upstream.URL).Routes()) + t.Cleanup(srv.Close) + return srv.URL, &paths } func readNPMTestBody(t *testing.T, r io.Reader) string {