From a68add48140c07e62a75fa3d32ac59728c87fbee Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 17:48:50 +0000 Subject: [PATCH] Let a test give the handlers the upstream fetcher handlers.Params gets an optional Fetcher, marked optional for fx. When the app provides one, the handlers pass it to the image service, whose Fetcher option already existed for tests, instead of letting it build its own from the config. pixad provides none, so production builds its fetcher from the config exactly as before. A new test builds the handlers in an fx app that provides no fetcher, as pixad does, and checks that an image from an allowlisted localhost is refused with 403 by the fetcher the handlers built. Model: opus-5-5 --- internal/handlers/fetcher_internal_test.go | 49 ++++++++++++++++++++++ internal/handlers/handlers.go | 11 ++++- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 internal/handlers/fetcher_internal_test.go diff --git a/internal/handlers/fetcher_internal_test.go b/internal/handlers/fetcher_internal_test.go new file mode 100644 index 0000000..c1f41d8 --- /dev/null +++ b/internal/handlers/fetcher_internal_test.go @@ -0,0 +1,49 @@ +package handlers + +import ( + "net/http" + "path/filepath" + "testing" + + "github.com/go-chi/chi/v5" + "go.uber.org/fx" + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/healthcheck" + "sneak.berlin/go/pixa/internal/logger" +) + +// TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided builds the handlers as +// pixad does, in an fx app that provides no fetcher, and requests an image +// from localhost, which is on the allowlist. The fetcher the handlers build +// from the config refuses localhost, so the answer is 403. +func TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided(t *testing.T) { + t.Parallel() + + stateDir := t.TempDir() + cfg := &config.Config{ + SigningKey: testSigningKey, + StateDir: stateDir, + DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"), + AllowlistHosts: []string{"localhost"}, + } + + var h *Handlers + + app := fxtest.New(t, + fx.Supply(cfg), + fx.Provide(globals.New, logger.New, database.New, healthcheck.New, New), + fx.Populate(&h), + ) + app.RequireStart() + t.Cleanup(app.RequireStop) + + r := chi.NewRouter() + r.Get("/v1/image/*", h.HandleImage()) + + rec := sendGet(t, r, photoURL("localhost")) + checkErrorBody(t, rec, http.StatusForbidden, "forbidden") +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 489e3c3..add7f98 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -27,6 +27,11 @@ type Params struct { Healthcheck *healthcheck.Healthcheck Database *database.Database Config *config.Config + + // Fetcher, when provided, fetches upstream images in place of the + // fetcher the handlers build from the config. Only tests provide one; + // pixad does not. + Fetcher httpfetcher.Fetcher `optional:"true"` } // Handlers provides HTTP request handlers. @@ -35,6 +40,7 @@ type Handlers struct { hc *healthcheck.Healthcheck db *database.Database config *config.Config + fetcher httpfetcher.Fetcher imgSvc *imgcache.Service imgCache *imgcache.Cache sessMgr *session.Manager @@ -54,6 +60,7 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) { hc: params.Healthcheck, db: params.Database, config: params.Config, + fetcher: params.Fetcher, csrfProtect: csrfProtect, } @@ -122,10 +129,12 @@ func (s *Handlers) initImageService() error { fetcherCfg.MaxConnections = s.config.UpstreamConnections fetcherCfg.BlockedNetworks = s.config.BlockedNetworks - // Create the service + // Create the service. With no fetcher provided, it builds its own from + // fetcherCfg. svc, err := imgcache.NewService(&imgcache.ServiceConfig{ Cache: cache, FetcherConfig: fetcherCfg, + Fetcher: s.fetcher, SigningKey: s.config.SigningKey, Allowlist: s.config.AllowlistHosts, MaxConcurrentProcessing: s.config.MaxConcurrentProcessing,