Sandbox upstream content on package routes

internal/server/detached_fetch_test.go

@@ -36,6 +36,12 @@ func TestServerKeepsFetchAfterClientHangsUp(t *testing.T) {
 	}
 }
 
+// slowRunnerBudget bounds the steps that only need to finish eventually: the
+// upstream fetch starting, and the follow-up request being answered. What the
+// test checks is the outcome, not the speed, and on a slow CI runner a
+// shorter budget made it fail with nothing wrong.
+const slowRunnerBudget = 30 * time.Second
+
 func testServerKeepsFetchAfterClientHangsUp(t *testing.T) {
 	const pkgPath = "/core/os/x86_64/big-1.0-1-x86_64.pkg.tar.zst"
 	release := make(chan struct{})
@@ -93,7 +99,7 @@ func testServerKeepsFetchAfterClientHangsUp(t *testing.T) {
 
 	// The client gives up while the upstream is still sending nothing.
 	impatient := &http.Client{Timeout: 300 * time.Millisecond}
-	deadline := time.Now().Add(5 * time.Second)
+	deadline := time.Now().Add(slowRunnerBudget)
 	for hits.Load() == 0 {
 		if _, err := impatient.Get(target); err == nil {
 			t.Fatal("request succeeded while the upstream was held")
@@ -106,7 +112,7 @@ func testServerKeepsFetchAfterClientHangsUp(t *testing.T) {
 	time.Sleep(100 * time.Millisecond)
 	releaseOnce.Do(func() { close(release) })
 
-	client := &http.Client{Timeout: 5 * time.Second}
+	client := &http.Client{Timeout: slowRunnerBudget}
 	resp, err := client.Get(target)
 	if err != nil {
 		t.Fatalf("follow-up request: %v", err)

internal/server/middleware.go

@@ -139,3 +139,19 @@ func rejectQueryInPath(next http.Handler) http.Handler {
 		next.ServeHTTP(w, r)
 	})
 }
+
+// sandboxUpstreamContent keeps package-route responses from running as pages
+// on the proxy's origin, which also serves /ui. Their bodies, and usually
+// their Content-Type, come from upstreams or from whoever pushed a hosted
+// package, so an artifact served as text/html or image/svg+xml could otherwise
+// run scripts with access to the UI. nosniff stops browsers from guessing a
+// renderable type, and a sandbox policy runs anything that does render with
+// scripts disabled and an opaque origin. Package managers ignore both
+// headers. Archive files browsed from /ui get the same headers in browse.go.
+func sandboxUpstreamContent(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("X-Content-Type-Options", "nosniff")
+		w.Header().Set("Content-Security-Policy", "sandbox")
+		next.ServeHTTP(w, r)
+	})
+}

internal/server/server.go

@@ -454,7 +454,7 @@ func (s *Server) mountProtocolHandlers(r chi.Router, proxy *handler.Proxy) {
 	rpmHandler := handler.NewRPMHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.RPM)
 	genericHandler := handler.NewGenericHandler(proxy, s.cfg.Upstream.Generic, s.cfg.Upstream.GenericArtifacts)
 
-	r = r.With(rejectQueryInPath)
+	r = r.With(sandboxUpstreamContent, rejectQueryInPath)
 	r.Mount("/npm", http.StripPrefix("/npm", npmHandler.Routes()))
 	r.Mount("/cargo", http.StripPrefix("/cargo", cargoHandler.Routes()))
 	r.Mount("/gem", http.StripPrefix("/gem", gemHandler.Routes()))

internal/server/untrusted_content_test.go

@@ -0,0 +1,68 @@
+package server
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/git-pkgs/proxy/internal/config"
+	"github.com/git-pkgs/proxy/internal/database"
+	"github.com/git-pkgs/proxy/internal/handler"
+	"github.com/git-pkgs/proxy/internal/storage"
+	"github.com/git-pkgs/registries/fetch"
+	"github.com/go-chi/chi/v5"
+)
+
+// TestPackageRoutesSandboxUpstreamContent checks that responses on package
+// routes cannot run as pages on the proxy's origin, which also serves /ui.
+// Their bodies and Content-Type come from upstreams, or from whoever pushed a
+// hosted package, so an archive served as text/html would otherwise run its
+// scripts with access to the UI.
+func TestPackageRoutesSandboxUpstreamContent(t *testing.T) {
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/html")
+		_, _ = io.WriteString(w, "<script>alert(document.domain)</script>")
+	}))
+	t.Cleanup(upstream.Close)
+
+	db, err := database.Create(t.TempDir() + "/test.db")
+	if err != nil {
+		t.Fatalf("creating database: %v", err)
+	}
+	t.Cleanup(func() { _ = db.Close() })
+	store, err := storage.OpenBucket(context.Background(), "file://"+t.TempDir())
+	if err != nil {
+		t.Fatalf("opening storage: %v", err)
+	}
+	fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0))
+	t.Cleanup(func() { _ = fetcher.Close() })
+	proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), slog.New(slog.NewTextHandler(io.Discard, nil)))
+	proxy.HTTPClient = upstream.Client()
+
+	cfg := config.Default()
+	cfg.BaseURL = "http://proxy.example"
+	cfg.Upstream.Generic = map[string]string{"site": upstream.URL}
+	cfg.Upstream.APK = map[string]string{"alpine": upstream.URL}
+	s := &Server{cfg: cfg}
+	r := chi.NewRouter()
+	s.mountProtocolHandlers(r, proxy)
+
+	for _, target := range []string{
+		"/generic/site/files/page-1.0.html",         // cached artifact
+		"/generic/site/files/page-1.0.html",         // the same, from the cache
+		"/apk/alpine/v3.22/main/x86_64/README.html", // relayed as is
+		"/npm/does-not-exist%2F..",                  // an error response
+	} {
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil))
+		if got := w.Header().Get("X-Content-Type-Options"); got != "nosniff" {
+			t.Errorf("%s (status %d): X-Content-Type-Options = %q, want nosniff", target, w.Code, got)
+		}
+		if got := w.Header().Get("Content-Security-Policy"); got != "sandbox" {
+			t.Errorf("%s (status %d): Content-Security-Policy = %q, want sandbox", target, w.Code, got)
+		}
+	}
+}