diff --git a/internal/server/detached_fetch_test.go b/internal/server/detached_fetch_test.go index 557de510..c18a4a63 100644 --- a/internal/server/detached_fetch_test.go +++ b/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) diff --git a/internal/server/middleware.go b/internal/server/middleware.go index 4a62d465..f74590a9 100644 --- a/internal/server/middleware.go +++ b/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) + }) +} diff --git a/internal/server/server.go b/internal/server/server.go index 5b35435c..2907f04c 100644 --- a/internal/server/server.go +++ b/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())) diff --git a/internal/server/untrusted_content_test.go b/internal/server/untrusted_content_test.go new file mode 100644 index 00000000..59824024 --- /dev/null +++ b/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, "") + })) + 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) + } + } +}