Author SHA1 Message Date
sneak 6453b9a125 Say the source host is signed as the URL writes it (part of #246)
check / check (push) Waiting to run
The Signature Specification read as though the hostname were signed
with ":" and the port appended. The code signs the host segment as the
URL writes it up to the first "/", lower-cased, so IPv6 brackets and a
":" with or without a port are kept. The README line, the TODO.md entry
and the parser comment now say so.

Model: opus-5-5
2026-10-15 10:03:47 +00:00
sneak 589318db97 Read the source host's port from the parsed URL (part of #246)
check / check (push) Waiting to run
net.SplitHostPort fails on an IPv6 address written without brackets,
so with 2001:db8::1 allowlisted, 2001:db8::1:8443 and 2001:db8::1:
counted as having no port and were served without a signature, though
the fetcher connects to port 8443. ValidateRequest now takes a host to
have a port when the parsed URL's Port() is set or the host ends in
":", as Go's URL parser and so the fetcher read it.

Model: opus-5-5
2026-10-15 07:38:14 +00:00
sneak b71f2958db Test that an unbracketed IPv6 host with a port needs a signature (part of #246)
With 2001:db8::1 allowlisted, Go's URL parser, which the fetcher uses,
reads 2001:db8::1:8443 as 2001:db8::1 with port 8443, and
2001:db8::1: as 2001:db8::1 with an empty port. Both must need a
signature. The test fails until the port check reads the port as the
parsed URL does.

Model: opus-5-5
2026-10-15 07:38:14 +00:00
sneak ad47aab290 Lower-case the image URL's host and require a signature for a port (closes #246)
With cdn.example.com allowlisted, CDN.Example.com and
cdn.example.com:8443 passed the allowlist too, and each spelling had its
own cache entries and its own connections under
upstream_connections_per_host. The image URL parser now lower-cases the
host and refuses one holding "@" with ErrInvalidHost (400).
ValidateRequest counts a host followed by ":", with or without a port,
as not allowlisted, so it needs a signature. That check is not in
allowlist.IsAllowed, which also matches referer_blocklist. A URL is now
signed over the lower-cased host; README.md says so.

Model: opus-5-5
2026-10-15 07:38:14 +00:00
sneak 9d9fb52c97 Test that the image URL's host is one spelling (part of #246)
The image URL parser must lower-case the host and refuse one holding
"@", which a URL reads as a user name and password before the host;
ErrInvalidHost is added for that refusal. A host with a port, or a ":"
with no port, must match no allowlist entry, so its request needs a
signature. All three tests fail until the fix.

Model: opus-5-5
2026-10-15 07:37:40 +00:00
9 changed files with 193 additions and 245 deletions
+14 -12
View File
@@ -198,11 +198,9 @@ path under `/v1/` answers 200, in maintenance mode too.
redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
address in a blocked network (see `blocked_networks`); 429 over
`image_requests_per_minute` (below); 415 when the upstream answers with a
`Content-Type` that is not one of the source formats (below), with a body
whose first bytes are not those of the format its `Content-Type` names, or
with an image whose first bytes libvips reads as another format; 502 when the
upstream answered with an error status, and for 5 minutes after that, or after
a body whose first bytes are not those of its `Content-Type`, for the same
`Content-Type` that is not one of the source formats (below), or with an image
whose first bytes libvips reads as another format; 502 when the upstream
answered with an error status, and for 5 minutes after that for the same
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
failure.
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
@@ -281,12 +279,8 @@ above 8192.
The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
`Content-Type` must name its format, and the image's first bytes must match it.
An upstream answering with any other `Content-Type`, SVG included, is refused
with 415. So is a body whose first bytes are not those of the format its
`Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image,
or an empty body. pixa then does not fetch that source URL again for 5 minutes,
and answers 502 for it meanwhile, as after an upstream error status. An image
whose first bytes libvips reads as another format, such as an SVG whose first
bytes pass the check for AVIF, is refused with 415 too.
with 415, and so is an image whose first bytes libvips reads as another format,
such as an SVG whose first bytes pass the check for AVIF.
With the format `auto`, pixa chooses the format for each request from its
`Accept` header, in this order:
@@ -424,7 +418,9 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
Where:
- `host` — source origin hostname (e.g. `cdn.example.com`)
- `host` — source host as the URL writes it up to the first `/`, in lower case,
keeping an IPv6 address's brackets and any `:` and port after it (e.g.
`cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
- `query` — source query string, empty string if none, with `%` and `:` written
@@ -479,6 +475,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
dots, with at most one leading dot) nor an IP address, such as one with a port
or a `*.` wildcard, aborts startup.
pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
`CDN.Example.com` is matched, fetched, cached and counted against
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
even when the host is allowlisted. A host holding `@` is refused with 400.
### Configuration
Every setting can be given as an environment variable, in a YAML config file
+11 -9
View File
@@ -30,15 +30,17 @@ P2: security: per-origin rate limiting
# Completed Steps
- 2026-10-15 a source whose first bytes do not match its `Content-Type` answers
415 (closes #253): a body whose first bytes are not those of the format its
`Content-Type` names, such as a JPEG sent as `image/png`, text, or a body
under 12 bytes, answered 500 on both image routes and was logged as an error
of pixa's, and each request for it fetched the whole body again. Both routes
now answer 415, and the image service logs it at warning level as a refused
source. It also gets a negative cache entry, as an upstream error status does,
so the source URL is not fetched again for 5 minutes and answers 502
meanwhile.
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
needs a signature (closes #246): with `cdn.example.com` allowlisted,
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
each spelling was fetched and cached on its own and had connections of its own
under `upstream_connections_per_host`. The image URL parser now lower-cases
the host and refuses one holding `@`, which a URL reads as a user name and
password, with 400. A host with a port, or with a `:` and no port, needs a
signature even when it is allowlisted. That is checked in the image service,
not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
`Referer` is blocked whatever its port. A URL is signed over its host as it
writes it, in lower case, with any IPv6 brackets, `:` and port kept.
- 2026-10-15 a `:` in the source path or query no longer moves where the path
ends and the query begins (closes #247): the signed data and the cache key
joined host, path and query with `:` and no escaping, so a URL signed for
@@ -1,180 +0,0 @@
package handlers
import (
"bytes"
"encoding/json"
"image/color"
"log/slog"
"net/http"
"slices"
"testing"
"testing/fstest"
"time"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imgcache"
)
// mismatchedSourcePath is the path of the source that
// TestFirstBytesNotMatchingContentType serves on allowlistedHost. The mock
// fetcher names a file's Content-Type from its extension, so the source is
// sent as image/png.
const mismatchedSourcePath = "/images/source.png"
// newRoutesFetchingThrough returns both image routes, and the Handlers behind
// them, whose image service fetches through fetcher and has allowlistedHost on
// its allowlist. The handlers and the image service log to log.
func newRoutesFetchingThrough(
t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
) (*Handlers, http.Handler) {
t.Helper()
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(),
NegativeTTL: 5 * time.Minute,
})
if err != nil {
t.Fatalf("imgcache.NewCache() error = %v", err)
}
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache,
Fetcher: fetcher,
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
Logger: log,
})
if err != nil {
t.Fatalf("imgcache.NewService() error = %v", err)
}
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
h := &Handlers{log: log, imgSvc: svc, encGen: encGen}
r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage())
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
return h, r
}
// mismatchedSourcePlainURL returns the /v1/image/ URL of mismatchedSourcePath
// on allowlistedHost at 50x50 as a JPEG.
func mismatchedSourcePlainURL(*testing.T, *Handlers) string {
return "/v1/image/" + allowlistedHost + mismatchedSourcePath + "/50x50.jpeg"
}
// mismatchedSourceEncURL returns an encrypted /v1/e/ URL, made with h's
// generator, of mismatchedSourcePath on allowlistedHost at 50x50 as a JPEG.
func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
t.Helper()
token, err := h.encGen.Generate(&encurl.Payload{
SourceHost: allowlistedHost,
SourcePath: mismatchedSourcePath,
Width: 50,
Height: 50,
Format: imgcache.FormatJPEG,
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
return "/v1/e/" + token + "/img.jpg"
}
// loggedLevels reads the JSON log lines in logs and returns the messages
// logged at each level.
func loggedLevels(t *testing.T, logs *bytes.Buffer) map[string][]string {
t.Helper()
t.Logf("logged:\n%s", logs.String())
messages := make(map[string][]string)
dec := json.NewDecoder(logs)
for dec.More() {
var line struct {
Level string `json:"level"`
Msg string `json:"msg"`
}
err := dec.Decode(&line)
if err != nil {
t.Fatalf("decoding log line: %v", err)
}
messages[line.Level] = append(messages[line.Level], line.Msg)
}
return messages
}
// TestFirstBytesNotMatchingContentType checks that both image routes answer
// 415 for an upstream body whose first bytes are not those of the format its
// Content-Type names, and log it as a refused source, not as an error of
// pixa's. The next request for that source must answer 502 without fetching
// it again, as after an upstream error status.
func TestFirstBytesNotMatchingContentType(t *testing.T) {
t.Parallel()
bodies := map[string][]byte{
"JPEG sent as image/png": generateTestJPEG(t, 10, 10, color.Black),
"text sent as image/png": []byte("This is just text, not a PNG"),
"body too short to name a format": []byte("short"),
}
// The source's URL on each image route.
sourceURLs := map[string]func(*testing.T, *Handlers) string{
"plain URL": mismatchedSourcePlainURL,
"encrypted URL": mismatchedSourceEncURL,
}
for bodyName, body := range bodies {
for urlName, sourceURL := range sourceURLs {
t.Run(urlName+", "+bodyName, func(t *testing.T) {
t.Parallel()
fetcher := &countingFetcher{
Fetcher: httpfetcher.NewMock(fstest.MapFS{
allowlistedHost + mismatchedSourcePath: &fstest.MapFile{
Data: body,
},
}),
}
var logs bytes.Buffer
h, routes := newRoutesFetchingThrough(t, fetcher,
slog.New(slog.NewJSONHandler(&logs, nil)))
target := sourceURL(t, h)
checkErrorBody(t, sendGet(t, routes, target),
http.StatusUnsupportedMediaType, "unsupported source format")
levels := loggedLevels(t, &logs)
if len(levels[slog.LevelError.String()]) != 0 {
t.Errorf("logged at error level: %q",
levels[slog.LevelError.String()])
}
if !slices.Contains(levels[slog.LevelWarn.String()], "source refused") {
t.Error(`no "source refused" line logged at warning level`)
}
checkErrorBody(t, sendGet(t, routes, target),
http.StatusBadGateway, "upstream error")
if n := fetcher.fetches.Load(); n != 1 {
t.Errorf("upstream fetched %d times, want 1", n)
}
})
}
}
}
-13
View File
@@ -15,7 +15,6 @@ import (
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/magic"
)
// HandleImage handles the main image proxy route:
@@ -221,18 +220,6 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
func (s *Handlers) respondImageError(
w http.ResponseWriter, req *imgcache.ImageRequest, err error,
) {
// An upstream body whose first bytes are not those of the format its
// Content-Type names is the upstream's error, not pixa's: the image
// service has logged it as a refused source.
if errors.Is(err, magic.ErrMagicByteMismatch) ||
errors.Is(err, magic.ErrUnknownFormat) ||
errors.Is(err, magic.ErrNotEnoughData) {
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
return
}
s.log.Error("failed to get image",
"host", req.SourceHost,
"path", req.SourcePath,
+1 -5
View File
@@ -15,7 +15,6 @@ import (
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/magic"
)
// HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
@@ -163,10 +162,7 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
s.respondError(w, "server busy, try again later",
http.StatusServiceUnavailable)
case errors.Is(err, httpfetcher.ErrInvalidContentType),
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat),
errors.Is(err, magic.ErrMagicByteMismatch),
errors.Is(err, magic.ErrUnknownFormat),
errors.Is(err, magic.ErrNotEnoughData):
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
case errors.Is(err, imageprocessor.ErrOutputTooLarge):
+19 -24
View File
@@ -9,6 +9,7 @@ import (
"log/slog"
"net/url"
"runtime/debug"
"strings"
"time"
"github.com/dustin/go-humanize"
@@ -218,7 +219,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
return fmt.Errorf("invalid source URL: %w", err)
}
if s.allowlist.IsAllowed(parsedURL) {
// 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
}
@@ -438,7 +448,13 @@ func (s *Service) fetchAndProcess(
if err != nil {
// Store negative cache for certain errors
if isNegativeCacheable(err) {
s.storeNegative(ctx, req, extractStatusCode(err), err)
statusCode := extractStatusCode(err)
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
if storeErr != nil {
s.log.Warn("failed to store negative cache entry",
"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
}
}
return nil, fmt.Errorf("upstream fetch failed: %w", err)
@@ -484,16 +500,9 @@ func (s *Service) fetchAndProcess(
"cipher", fetchResult.TLSCipherSuite,
)
// Validate magic bytes match content type. A body that does not is the
// upstream's error, so it gets a negative cache entry, as an upstream
// error status does.
// Validate magic bytes match content type
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
if err != nil {
s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
"host", req.SourceHost, "path", req.SourcePath,
"content_type", fetchResult.ContentType, "error", err)
s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err)
return nil, fmt.Errorf("content validation failed: %w", err)
}
@@ -509,19 +518,6 @@ func (s *Service) fetchAndProcess(
)
}
// storeNegative stores a negative cache entry for req's source, whose fetch
// failed with err, so the source is not fetched again until the entry
// expires. A failure to store the entry is only logged.
func (s *Service) storeNegative(
ctx context.Context, req *ImageRequest, statusCode int, err error,
) {
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
if storeErr != nil {
s.log.Warn("failed to store negative cache entry",
"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
}
}
// processedVariant is a variant as processAndStore made it. Each request that
// shared its processing serves it through a reader of its own.
type processedVariant struct {
@@ -632,7 +628,6 @@ func signatureRequest(req *ImageRequest) *signature.Request {
const (
httpStatusBadGateway = 502
httpStatusInternalError = 500
httpStatusUnsupportedMediaType = 415
)
// isNegativeCacheable returns true if the error should be cached.
@@ -250,6 +250,97 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
}
}
// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
// followed by a port, or by a colon with no port, matches no allowlist entry,
// so that a request for it without a signature is refused, while the same
// host with no port is allowlisted.
func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
)
tests := []struct {
host string
allowlisted bool
}{
{testHostCDN, true},
{"cdn.example.com:8443", false},
{"cdn.example.com:443", false},
{"cdn.example.com:", false},
{"203.0.113.5", true},
{"203.0.113.5:8080", false},
{"[2001:db8::1]", true},
{"[2001:db8::1]:8443", false},
}
for _, tt := range tests {
t.Run(tt.host, func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: tt.host,
SourcePath: testPathCat,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
tt.host, err)
if allowlisted := err == nil; allowlisted != tt.allowlisted {
t.Errorf("host %q served without a signature = %v, want %v",
tt.host, allowlisted, tt.allowlisted)
}
})
}
}
// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
// an allowlisted IPv6 address written without brackets and followed by a
// port, or by a colon with no port, needs a signature. Go's URL parser, which
// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
// port 8443.
func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
t *testing.T,
) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist("2001:db8::1"),
)
for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
t.Run(host, func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: host,
SourcePath: testPathCat,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
host, err)
if err == nil {
t.Errorf("host %q served without a signature, want refused",
host)
}
})
}
}
func TestService_Get_InvalidFile(t *testing.T) {
t.Parallel()
+12
View File
@@ -15,6 +15,7 @@ import (
var (
ErrInvalidPath = errors.New("invalid image path")
ErrMissingHost = errors.New("missing source host")
ErrInvalidHost = errors.New("invalid source host")
ErrMissingSize = errors.New("missing size specification")
ErrInvalidSize = errors.New("invalid size format")
ErrInvalidFormat = errors.New("invalid or unsupported format")
@@ -143,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
return nil, ErrMissingHost
}
// A URL reads what comes before "@" as a user name and password, and the
// rest as the host
if strings.Contains(host, "@") {
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
}
// Host names do not depend on letter case, so the allowlist, the upstream
// connection limits, the cache and the signature see one spelling of each
// host
host = strings.ToLower(host)
// Extract query string if present in path
if qIndex := strings.Index(path, "?"); qIndex != -1 {
query = path[qIndex+1:]
@@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) {
}
}
// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
// that every spelling of its letter case is the same host to the allowlist,
// the upstream connection limits and the cache, and that the path keeps its
// case.
func TestParseImagePath_LowerCasesHost(t *testing.T) {
t.Parallel()
got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
if err != nil {
t.Fatalf("ParseImagePath() error = %v", err)
}
t.Logf("parsed host %q, path %q", got.Host, got.Path)
if got.Host != testHostCDN {
t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
}
if got.Path != "/Photos/Cat.jpg" {
t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
}
}
// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
// refused: a URL reads what comes before the "@" as a user name and password
// and the rest as the host.
func TestParseImagePath_RefusesUserinfo(t *testing.T) {
t.Parallel()
for _, input := range []string{
"user@cdn.example.com/photos/cat.jpg/800x600.webp",
"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
} {
got, err := ParseImagePath(input)
t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
if !errors.Is(err, ErrInvalidHost) {
t.Errorf("ParseImagePath(%q) error = %v, want %v",
input, err, ErrInvalidHost)
}
}
}
func TestParsedURL_ToImageRequest(t *testing.T) {
t.Parallel()