Harden nuget per-request rewrite and fix HEAD length

- set Cache-Control no-store and Vary on rewritten responses - accept only http/https X-Forwarded-Proto - reject invalid Host headers and JSON-escape the injected base URL - route remote HEAD and serve rewritten HEAD via the GET path - classify snupkg, readme and icon as immutable
2026-10-17 12:39:02 +11:00
parent bc68eb75fc
commit 3b69b58ba3
5 changed files with 194 additions and 11 deletions
@@ -6,7 +6,10 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"net"
 	"net/http"
+	"strconv"
+	"strings"

 	"github.com/go-chi/chi/v5"

@@ -16,6 +19,7 @@ import (
 	"git.unkin.net/unkin/artifactapi/internal/proxy"
 	"git.unkin.net/unkin/artifactapi/internal/storage"
 	"git.unkin.net/unkin/artifactapi/internal/virtual"
+	"git.unkin.net/unkin/artifactapi/pkg/models"
 )

 type ProxyHandler struct {
@@ -41,6 +45,7 @@ func NewProxyHandler(engine *proxy.Engine, virtualEngine *virtual.Engine, db *da
 func (h *ProxyHandler) Routes() chi.Router {
 	r := chi.NewRouter()
 	r.Get("/remote/{remoteName}/*", h.handleProxy)
+	r.Head("/remote/{remoteName}/*", h.handleProxyHead)
 	r.Get("/local/{localName}/*", h.handleLocal)
 	r.Get("/virtual/{virtualName}/*", h.handleVirtual)
 	return r
@@ -107,13 +112,11 @@ func (h *ProxyHandler) handleProxy(w http.ResponseWriter, r *http.Request) {
 	defer result.Reader.Close()

 	var body io.Reader = result.Reader
-	if sr, ok := prov.(provider.ServeRewriter); ok && prov.Classify(path) == provider.Mutable {
-		raw, err := io.ReadAll(result.Reader)
-		if err != nil {
-			http.Error(w, "bad gateway", http.StatusBadGateway)
+	if sr, ok := serveRewriter(prov, path); ok {
+		raw, ok := rewriteServed(w, r, sr, *remote, result.Reader)
+		if !ok {
 			return
 		}
-		raw = sr.RewriteServed(raw, *remote, fmt.Sprintf("%s://%s", scheme(r), r.Host))
 		body, result.Size = bytes.NewReader(raw), int64(len(raw))
 	}

@@ -142,6 +145,13 @@ func (h *ProxyHandler) handleProxyHead(w http.ResponseWriter, r *http.Request) {
 		return
 	}

+	// Rewritten bodies differ in size from the cached object, so HEAD takes the
+	// GET path; net/http discards the body for HEAD.
+	if _, ok := serveRewriter(prov, path); ok {
+		h.handleProxy(w, r)
+		return
+	}
+
 	result, err := h.engine.Head(r.Context(), *remote, path, prov)
 	if err != nil {
 		var proxyErr *proxy.ProxyError
@@ -253,8 +263,63 @@ func scheme(r *http.Request) string {
 	if r.TLS != nil {
 		return "https"
 	}
-	if fwd := r.Header.Get("X-Forwarded-Proto"); fwd != "" {
+	if fwd := r.Header.Get("X-Forwarded-Proto"); fwd == "http" || fwd == "https" {
 		return fwd
 	}
 	return "http"
 }
+
+// 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)
+		return nil, false
+	}
+	raw, err := io.ReadAll(src)
+	if err != nil {
+		http.Error(w, "bad gateway", http.StatusBadGateway)
+		return nil, false
+	}
+	raw = sr.RewriteServed(raw, remote, fmt.Sprintf("%s://%s", scheme(r), r.Host))
+	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
+}
+
+func serveRewriter(prov provider.Provider, path string) (provider.ServeRewriter, bool) {
+	sr, ok := prov.(provider.ServeRewriter)
+	return sr, ok && prov.Classify(path) == provider.Mutable
+}
+
+// validHost accepts host[:port] where host is a DNS name or IP literal.
+func validHost(hostport string) bool {
+	host, port := hostport, ""
+	if h, p, err := net.SplitHostPort(hostport); err == nil {
+		host, port = h, p
+	} else if strings.HasPrefix(hostport, "[") {
+		return strings.HasSuffix(hostport, "]") && net.ParseIP(hostport[1:len(hostport)-1]) != nil
+	}
+	if port != "" {
+		if n, err := strconv.Atoi(port); err != nil || n < 1 || n > 65535 {
+			return false
+		}
+	}
+	if net.ParseIP(host) != nil {
+		return true
+	}
+	if strings.HasPrefix(hostport, "[") {
+		return false
+	}
+	if host == "" || len(host) > 253 {
+		return false
+	}
+	for _, label := range strings.Split(strings.TrimSuffix(host, "."), ".") {
+		if label == "" || len(label) > 63 || label[0] == '-' || label[len(label)-1] == '-' ||
+			strings.Trim(strings.ToLower(label), "abcdefghijklmnopqrstuvwxyz0123456789-_") != "" {
+			return false
+		}
+	}
+	return true
+}
@@ -0,0 +1,74 @@
+package v1
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"git.unkin.net/unkin/artifactapi/internal/provider/nuget"
+	"git.unkin.net/unkin/artifactapi/pkg/models"
+)
+
+func TestValidHost(t *testing.T) {
+	for _, h := range []string{"aa.example", "aa.example:8443", "artifactapi.k8s.syd1.au.unkin.net", "10.0.0.1:80", "[::1]:8080", "[::1]", "::1", "localhost", "my_host.local"} {
+		if !validHost(h) {
+			t.Errorf("validHost(%q) = false, want true", h)
+		}
+	}
+	for _, h := range []string{"", "evil\".example", "a b", "aa.example:0", "aa.example:99999", "aa.example:x", "-bad.example", "a..b", "[::1", "[nope]:80", "a/b", "a<script>", strings.Repeat("a", 64) + ".example"} {
+		if validHost(h) {
+			t.Errorf("validHost(%q) = true, want false", h)
+		}
+	}
+}
+
+var nugetOrg = models.Remote{Name: "nuget-org", BaseURL: "https://api.nuget.org/v3/index.json"}
+
+const serviceIndex = `{"resources":[{"@id":"https://api.nuget.org/v3-flatcontainer/"}]}`
+
+func rewrite(t *testing.T, host, proto string) (*httptest.ResponseRecorder, string, bool) {
+	t.Helper()
+	r := httptest.NewRequest(http.MethodGet, "/api/v1/remote/nuget-org/index.json", nil)
+	r.Host = host
+	if proto != "" {
+		r.Header.Set("X-Forwarded-Proto", proto)
+	}
+	w := httptest.NewRecorder()
+	raw, ok := rewriteServed(w, r, &nuget.Provider{}, nugetOrg, strings.NewReader(serviceIndex))
+	return w, string(raw), ok
+}
+
+func TestRewriteServedHeaders(t *testing.T) {
+	w, body, ok := rewrite(t, "aa.example:8443", "https")
+	if !ok {
+		t.Fatal("valid request rejected")
+	}
+	if want := `{"resources":[{"@id":"https://aa.example:8443/api/v1/remote/nuget-org/v3-flatcontainer/"}]}`; body != want {
+		t.Errorf("body = %s, want %s", body, want)
+	}
+	if got := w.Header().Get("Cache-Control"); got != "no-store" {
+		t.Errorf("Cache-Control = %q, want no-store", got)
+	}
+	if got := w.Header().Get("Vary"); got != "Host, X-Forwarded-Proto" {
+		t.Errorf("Vary = %q", got)
+	}
+	if got := w.Header().Get("Content-Length"); got != "91" || len(body) != 91 {
+		t.Errorf("Content-Length = %q, body len %d", got, len(body))
+	}
+}
+
+func TestRewriteServedIgnoresBogusProto(t *testing.T) {
+	_, plain, _ := rewrite(t, "aa.example", "")
+	_, bogus, _ := rewrite(t, "aa.example", "javascript")
+	if bogus != plain || !strings.Contains(bogus, `"http://aa.example/`) {
+		t.Errorf("bogus X-Forwarded-Proto changed the body: %s", bogus)
+	}
+}
+
+func TestRewriteServedRejectsInvalidHost(t *testing.T) {
+	w, _, ok := rewrite(t, `evil"},{"x":"`, "")
+	if ok || w.Code != http.StatusBadRequest {
+		t.Errorf("invalid host: ok=%v code=%d, want rejected with 400", ok, w.Code)
+	}
+}
@@ -14,6 +14,12 @@ func TestScheme(t *testing.T) {
 	if got := scheme(r); got != "https" {
 		t.Errorf("X-Forwarded-Proto scheme = %q, want https", got)
 	}
+	for _, bogus := range []string{"javascript", "HTTPS", "https, http", "ftp"} {
+		r := &http.Request{Header: http.Header{"X-Forwarded-Proto": {bogus}}}
+		if got := scheme(r); got != "http" {
+			t.Errorf("X-Forwarded-Proto %q scheme = %q, want http", bogus, got)
+		}
+	}
 	if got := scheme(&http.Request{Header: http.Header{}}); got != "http" {
 		t.Errorf("default scheme = %q, want http", got)
 	}