From 8dcbe55936919a385ccc94405f6a8c7a6ad5aef3 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 21:46:17 +1100 Subject: [PATCH] Validate Host and mark npm packuments per-host uncacheable --- internal/api/v1/npm.go | 14 +++++++++- internal/api/v1/npm_handler_test.go | 40 +++++++++++++++++++++++++++++ internal/api/v1/proxy.go | 24 ++++++++++++++--- internal/virtual/npm_merger.go | 6 ++++- 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go index e5caf25..5927f7f 100644 --- a/internal/api/v1/npm.go +++ b/internal/api/v1/npm.go @@ -253,8 +253,12 @@ func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, npmError(w, http.StatusInternalServerError, err.Error()) return } + base, ok := requestBaseURL(w, r) + if !ok { + return + } read := func(hash string) ([]byte, error) { return h.readLocalBlob(r.Context(), hash) } - tarballBase := fmt.Sprintf("%s://%s/api/v1/local/%s/", Scheme(r), r.Host, remote.Name) + tarballBase := base + "/api/v1/local/" + remote.Name + "/" doc, err := buildNPMPackument(pkg, files, read, tarballBase) if err != nil { npmError(w, http.StatusInternalServerError, err.Error()) @@ -264,12 +268,20 @@ func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, npmError(w, http.StatusNotFound, "not found") return } + setNPMPerHostHeaders(w) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Artifact-Source", "local") w.WriteHeader(http.StatusOK) _ = json.NewEncoder(w).Encode(doc) } +// setNPMPerHostHeaders keeps shared caches off packuments whose tarball URLs +// are built from the request host and Accept (abbreviated vs full). +func setNPMPerHostHeaders(w http.ResponseWriter) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Vary", "Host, X-Forwarded-Proto, Accept") +} + // buildNPMPackument returns nil when pkg has no published versions. func buildNPMPackument(pkg string, files []database.LocalFile, read func(hash string) ([]byte, error), tarballBase string) (map[string]any, error) { prefix := pkg + "/-/" diff --git a/internal/api/v1/npm_handler_test.go b/internal/api/v1/npm_handler_test.go index 8b21c09..95c8f3b 100644 --- a/internal/api/v1/npm_handler_test.go +++ b/internal/api/v1/npm_handler_test.go @@ -18,6 +18,7 @@ import ( "git.unkin.net/unkin/artifactapi/internal/database" "git.unkin.net/unkin/artifactapi/internal/storage" "git.unkin.net/unkin/artifactapi/internal/testsupport" + "git.unkin.net/unkin/artifactapi/internal/virtual" "git.unkin.net/unkin/artifactapi/pkg/models" ) @@ -291,3 +292,42 @@ func TestNPMPublishHandler(t *testing.T) { } }) } + +func TestNPMPackumentPerHost(t *testing.T) { + h, srv := npmTestServer(t) + h.virtualEngine = virtual.NewEngine(h.db, nil) + h.virtualEngine.NPMLocal = h.LocalNPMPackument + repo := npmRepo(t, h, models.PackageNPM) + virt := "v" + repo + if err := h.db.CreateVirtual(context.Background(), &models.Virtual{Name: virt, PackageType: models.PackageNPM, Members: []string{repo}}); err != nil { + t.Fatal(err) + } + if code, b := npmDo(t, http.MethodPut, srv.URL+"/api/v1/local/"+repo+"/hostpkg", publishBody(t, "hostpkg", []byte("tgz"), publishOpts{})); code != http.StatusCreated { + t.Fatalf("publish = %d: %s", code, b) + } + routes := h.Routes() + get := func(path, host, proto string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, path, nil) + r.Host = host + if proto != "" { + r.Header.Set("X-Forwarded-Proto", proto) + } + w := httptest.NewRecorder() + routes.ServeHTTP(w, r) + return w + } + for _, path := range []string{"/local/" + repo + "/hostpkg", "/virtual/" + virt + "/hostpkg"} { + for _, host := range []string{"x@evil.com", "evil.example:"} { + if w := get(path, host, ""); w.Code != http.StatusBadRequest { + t.Errorf("%s Host %q = %d, want 400", path, host, w.Code) + } + } + w := get(path, "aa.example", "https") + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), `"tarball":"https://aa.example/api/v1/`) { + t.Fatalf("%s = %d: %s", path, w.Code, w.Body) + } + if cc, vary := w.Header().Get("Cache-Control"), w.Header().Get("Vary"); cc != "no-store" || vary != "Host, X-Forwarded-Proto, Accept" { + t.Errorf("%s Cache-Control %q Vary %q", path, cc, vary) + } + } +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 2d6f3b7..3ee4212 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -196,7 +196,10 @@ func (h *ProxyHandler) handleVirtual(w http.ResponseWriter, r *http.Request) { return } - proxyBaseURL := fmt.Sprintf("%s://%s", Scheme(r), r.Host) + proxyBaseURL, ok := requestBaseURL(w, r) + if !ok { + return + } loc, ok, err := h.virtualEngine.MemberRedirect(r.Context(), *virt, path, r.URL.RawQuery, proxyBaseURL) if err != nil { @@ -226,6 +229,9 @@ func (h *ProxyHandler) handleVirtual(w http.ResponseWriter, r *http.Request) { return } + if virt.PackageType == models.PackageNPM { + setNPMPerHostHeaders(w) + } w.Header().Set("Content-Type", contentType) w.Header().Set("X-Artifact-Source", "virtual") w.WriteHeader(http.StatusOK) @@ -303,8 +309,8 @@ func Scheme(r *http.Request) string { // rewriteServed rewrites a cached body for the requesting host and marks the // response uncacheable by shared caches, since it varies per Host. func rewriteServed(w http.ResponseWriter, r *http.Request, sr provider.ServeRewriter, remote models.Remote, src io.Reader) ([]byte, bool) { - if !validHost(r.Host) { - http.Error(w, "invalid host header", http.StatusBadRequest) + base, ok := requestBaseURL(w, r) + if !ok { return nil, false } raw, err := io.ReadAll(src) @@ -312,13 +318,23 @@ func rewriteServed(w http.ResponseWriter, r *http.Request, sr provider.ServeRewr http.Error(w, "bad gateway", http.StatusBadGateway) return nil, false } - raw = sr.RewriteServed(raw, remote, fmt.Sprintf("%s://%s", Scheme(r), r.Host)) + raw = sr.RewriteServed(raw, remote, base) w.Header().Set("Cache-Control", "no-store") w.Header().Set("Vary", "Host, X-Forwarded-Proto") w.Header().Set("Content-Length", strconv.Itoa(len(raw))) return raw, true } +// requestBaseURL returns the client-facing scheme://host, answering 400 on an +// invalid Host. +func requestBaseURL(w http.ResponseWriter, r *http.Request) (string, bool) { + if !validHost(r.Host) { + http.Error(w, "invalid host header", http.StatusBadRequest) + return "", false + } + return Scheme(r) + "://" + r.Host, true +} + // validHost accepts host[:port] where host is a DNS name or IP literal. func validHost(hostport string) bool { host, port := hostport, "" diff --git a/internal/virtual/npm_merger.go b/internal/virtual/npm_merger.go index 3c19fc9..844059f 100644 --- a/internal/virtual/npm_merger.go +++ b/internal/virtual/npm_merger.go @@ -200,7 +200,9 @@ func (e *Engine) FetchNPM(ctx context.Context, virt models.Virtual, path, proxyB } prefix := "npm\x00" + virt.Name + "\x00" tarballBase := strings.TrimRight(proxyBaseURL, "/") + "/api/v1/virtual/" + url.PathEscape(virt.Name) + "/" + inserted := false repo, err := e.merged(ctx, prefix+pkg+"\x00"+proxyBaseURL, func(ctx context.Context) (mergedRepo, error) { + inserted = true members, err := fetchMembers(virt, func(name string) (*NPMMember, error) { return e.npmMember(ctx, name, pkg) }) if err != nil { return nil, err @@ -214,7 +216,9 @@ func (e *Engine) FetchNPM(ctx context.Context, virt models.Virtual, path, proxyB if err != nil { return nil, "", err } - e.evictOldest(prefix, maxNPMPackuments) + if inserted { + e.evictOldest(prefix, maxNPMPackuments) + } if abbreviated { return repo.files()[npmAbbreviatedFile], NPMAbbreviatedType, nil }