Take the signature, the expiry and the URL scheme off the image request (closes #314)

The image request now holds only its source and its transform, exactly what
the variant key covers. The /v1/image/ handler checks the allowlist and
verifies the signature itself with Signer.Verify(req, sig, expires). Both
image handlers keep the URL's expiry as a local value for Cache-Control. The
image service holds no signer, allowlist or AllowHTTP, and ValidateRequest is
gone. The fetcher takes the source and builds its URL with Source.URL from its
own AllowHTTP, so no URL string is built and parsed again. chooseAutoFormat
returns the format it chooses instead of writing into the request. The signed
URL format and every route's answers are unchanged.

Model: opus-5-5

internal/imgcache/service.go

@@ -7,21 +7,17 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"net/url"
 	"runtime/debug"
-	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
 	"github.com/getsentry/sentry-go"
 	"github.com/go-chi/chi/v5/middleware"
 	"golang.org/x/sync/singleflight"
-	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/magic"
-	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // Service orchestrates the cache, fetcher, and processor.
@@ -29,10 +25,7 @@ type Service struct {
 	cache           *Cache
 	fetcher         httpfetcher.Fetcher
 	processor       *imageprocessor.ImageProcessor
-	signer          *signature.Signer
-	allowlist       *allowlist.HostAllowList
 	log             *slog.Logger
-	allowHTTP       bool
 	maxResponseSize int64
 	// variantsInProgress lets the requests that miss the same variant at the
 	// same time share one fetch and one transcode.
@@ -47,13 +40,6 @@ type ServiceConfig struct {
 	Fetcher httpfetcher.Fetcher
 	// Processor converts and resizes the images
 	Processor *imageprocessor.ImageProcessor
-	// SigningKey is the HMAC signing key (empty disables signing)
-	SigningKey string
-	// Allowlist is the list of hosts that don't require signatures
-	Allowlist []string
-	// AllowHTTP makes the service fetch sources over http instead of
-	// https (testing only), so the fetcher must allow http too
-	AllowHTTP bool
 	// MaxResponseSize is the largest source accepted, in bytes; a cached
 	// source larger than this is fetched again
 	MaxResponseSize int64
@@ -61,11 +47,8 @@ type ServiceConfig struct {
 	Logger *slog.Logger
 }
 
-// Static errors for service construction.
-var (
-	errCacheRequired      = errors.New("cache is required")
-	errSigningKeyRequired = errors.New("signing key is required")
-)
+// errCacheRequired is returned when the service is built without a cache.
+var errCacheRequired = errors.New("cache is required")
 
 // NewService creates a new image service.
 func NewService(cfg *ServiceConfig) (*Service, error) {
@@ -73,10 +56,6 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		return nil, errCacheRequired
 	}
 
-	if cfg.SigningKey == "" {
-		return nil, errSigningKeyRequired
-	}
-
 	log := cfg.Logger
 	if log == nil {
 		log = slog.Default()
@@ -93,10 +72,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		cache:           cfg.Cache,
 		fetcher:         cfg.Fetcher,
 		processor:       cfg.Processor,
-		signer:          signature.New(cfg.SigningKey),
-		allowlist:       allowlist.New(cfg.Allowlist),
 		log:             log,
-		allowHTTP:       cfg.AllowHTTP,
 		maxResponseSize: cfg.MaxResponseSize,
 	}, nil
 }
@@ -123,9 +99,6 @@ var ErrProcessingTimedOut = errors.New(
 func (s *Service) Get(
 	ctx context.Context, req *imagerequest.Request,
 ) (*ImageResponse, error) {
-	// Propagate AllowHTTP setting to the request
-	req.AllowHTTP = s.allowHTTP
-
 	cacheKey := imagerequest.CacheKey(req)
 
 	reader, size, contentType, err := s.cache.GetVariant(cacheKey)
@@ -169,33 +142,6 @@ func (s *Service) WaitForProcessing(ctx context.Context) int {
 	return s.processor.WaitForProcessing(ctx)
 }
 
-// ValidateRequest validates the request signature if required.
-func (s *Service) ValidateRequest(req *imagerequest.Request) error {
-	// Check if host is allowed (no signature required)
-	sourceURL := req.SourceURL()
-
-	parsedURL, err := url.Parse(sourceURL)
-	if err != nil {
-		return fmt.Errorf("invalid source URL: %w", err)
-	}
-
-	// An allowlist entry names no port, so a host followed by ":", with or
-	// without a port, needs a signature. The port is read from the parsed
-	// URL, as the fetcher reads it: in an IPv6 address written without
-	// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
-	// This is checked here and not in allowlist.IsAllowed, which also matches
-	// referer_blocklist, where a Referer must be blocked whatever its port.
-	hostHasPort := parsedURL.Port() != "" ||
-		strings.HasSuffix(parsedURL.Host, ":")
-
-	if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
-		return nil
-	}
-
-	// Signature required for non-allowed hosts
-	return s.signer.Verify(req)
-}
-
 // errPanicked is returned when processing a variant panicked.
 var errPanicked = errors.New("panic while processing image")
 
@@ -388,11 +334,10 @@ func (s *Service) fetchAndProcess(
 	}
 
 	// Fetch from upstream
-	sourceURL := req.SourceURL()
+	s.log.Debug("fetching from upstream", "host", req.Source.Host,
+		"path", req.Source.Path, "query", req.Source.Query)
 
-	s.log.Debug("fetching from upstream", "url", sourceURL)
-
-	fetchResult, err := s.fetcher.Fetch(ctx, sourceURL)
+	fetchResult, err := s.fetcher.Fetch(ctx, req.Source)
 	if err != nil {
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {