From 3b69b58ba32f77907fa1779e7fb96dd884219751 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:39:02 +1100 Subject: [PATCH 1/6] 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 --- internal/api/v1/proxy.go | 77 ++++++++++++++++++++++++--- internal/api/v1/rewrite_test.go | 74 +++++++++++++++++++++++++ internal/api/v1/scheme_test.go | 6 +++ internal/provider/nuget/nuget.go | 22 ++++++-- internal/provider/nuget/nuget_test.go | 26 +++++++++ 5 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 internal/api/v1/rewrite_test.go diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 7429007..94a7a4f 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -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 +} diff --git a/internal/api/v1/rewrite_test.go b/internal/api/v1/rewrite_test.go new file mode 100644 index 0000000..c2c30a9 --- /dev/null +++ b/internal/api/v1/rewrite_test.go @@ -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