next -> main (1.0.0 milestone) (#118)

Reviewed-on: #118

README.md

@@ -94,9 +94,38 @@ In-process caching of request-to-output mappings targets 1-5k r/s.
 
 Images are only fetched from origins using TLS with valid certificates.
 
+A request whose query string cannot be decoded, or gives any parameter more
+than once, is refused with 400.
+
 - `<format>`: one of `orig`, `png`, `jpeg`, `webp`
 - `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
 
+The login form (`POST /`) is limited to 5 attempts per minute per client
+address, counting an IPv6 client by its /64; an attempt over the limit is
+refused with 429 and a `Retry-After` header. Behind a reverse proxy the client
+address comes from `X-Forwarded-For` only when the proxy's address is in
+`trusted_proxies`; otherwise all users behind the proxy are counted as one
+client. With the default `trusted_proxies` (the RFC 1918 ranges), a client
+with a private address can choose the address it is counted by through its own
+`X-Forwarded-For`, whether it connects directly or through the proxy, because
+its own address is trusted too. Setting `trusted_proxies` to the proxy's own
+address closes this.
+
+### Image Metadata
+
+pixa decodes and re-encodes every image it serves, and removes all metadata from
+the output: EXIF (GPS position, camera make, model and serial number, capture
+time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be
+turned off.
+
+- The `orig` format means the source's own format, not the source's bytes: an
+  `orig` image is re-encoded and stripped like any other.
+- An image with an EXIF orientation is turned upright first, so it displays the
+  same without the tag; a requested size applies to the upright image.
+- An image with an ICC profile is converted to sRGB first, since clients show an
+  image with no profile as sRGB. Colours outside sRGB, such as the most
+  saturated ones in a Display P3 photo, are clipped.
+
 ### Source Hosts
 
 Source hosts may be allowlisted in the configuration. Non-allowlisted
@@ -124,12 +153,15 @@ Where:
 - `width` — requested width in pixels, `0` for original
 - `height` — requested height in pixels, `0` for original
 - `format` — output format (jpeg, png, webp, avif, gif, orig)
-- `expiration` — Unix timestamp when signature expires
+- `expiration` — the URL's `exp` query parameter, the Unix timestamp when
+  the signature expires; a request whose `exp` is not a whole number, an
+  empty `exp=` included, is refused with 400
 - `quality` — the URL's `q` query parameter, a whole number from 1 to 100,
   or `85` when the URL has no `q`; a request whose `q` is anything else is
   refused with 400
 - `fit` — the URL's `fit` query parameter (cover, contain, fill, inside,
-  outside), or `cover` when the URL has no `fit`
+  outside), or `cover` when the URL has no `fit`; a request whose `fit` is
+  anything else, an empty `fit=` included, is refused with 400
 
 **Example:** resize `https://cdn.example.com/photos/cat.jpg` to 800x600
 WebP with expiration 1704067200, default quality and fit:
@@ -199,7 +231,8 @@ Key settings in more detail:
   inside one of these ranges; the logged and login-recorded client
   address is then the rightmost forwarded entry that is not itself a
   trusted proxy. Otherwise the direct peer address is used and the header
-  is ignored, so a client connecting directly cannot spoof its address.
+  is ignored, so a client connecting directly from an address outside
+  these ranges cannot spoof its address.
   An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
   `172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
   proxy on a private network; an explicitly empty list (`[]`) trusts no

TODO.md

@@ -30,13 +30,46 @@ exhaustion
 
 # Completed Steps
 
+- 2026-09-28 strip metadata from processed images (closes #82): every output is
+  exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC
+  profile; the image is first turned upright with `AutoRotate` (before sizes are
+  worked out) and, when it has an ICC profile, converted to sRGB; the `orig`
+  format is re-encoded and stripped like any other, as pixa never serves the
+  source bytes; there is no setting to keep metadata; documented in `README.md`.
+- 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5
+  attempts per minute per client address, and an attempt over the limit is
+  refused with 429 and a `Retry-After` header; the address is the one
+  `internal/clientip` resolves through `trusted_proxies`, an IPv6 client is
+  counted by its /64, and an IPv4-mapped address as the IPv4 address it
+  carries; the limit is a `RateLimit` middleware in `internal/middleware` on
+  `github.com/go-chi/httprate`, which the image routes can reuse; the library
+  keeps counts for the current and the previous minute only; documented in
+  `README.md`.
+- 2026-09-28 refuse an unparseable `exp` on `/v1/image/` and log swallowed
+  cache errors (closes #72): an `exp` in the URL that is not a whole
+  number, an empty `exp=` included, is a 400 naming `exp` and the value,
+  instead of being ignored and answered with 401 as if the URL had no
+  `exp`; only an `exp` missing from the URL is unchanged; `README.md` says
+  so where it documents `exp`. A failed variant `.meta` write, source
+  metadata JSON write, `Stats` count query, stats counter update, negative
+  cache write or expired negative cache delete is now logged at `warn`
+  with the path or key and the error, and stays non-fatal.
+- 2026-09-28 refuse an empty `fit` on `/v1/image/` (closes #139): a
+  `fit` in the URL with an empty value (`fit=`) is a 400 naming `fit`,
+  instead of being served as `cover` and verified against a signature
+  made for `cover`; only a `fit` missing from the URL is still `cover`;
+  any other value still goes through the existing fit-mode check;
+  `README.md` says so where it documents `fit`.
 - 2026-09-28 refuse an invalid `q` on `/v1/image/` (closes #134): a `q`
   that is not a whole number from 1 to 100, an empty `q` included, is a
   400 naming `q` and the value, instead of being served at the default
   85; the route reads `q` with the generator's quality check
   (`parseFormInt` with `minQuality` and `maxQuality`); only a `q` missing
   from the URL is still 85; a query string that cannot be decoded, such
-  as `q=80%`, is a 400 showing it; `README.md` states the range.
+  as `q=80%`, is a 400 showing it; any query parameter given more than
+  once (`q`, `fit`, `sig`, `exp` alike) is a 400 naming it, so none is
+  read from its first value only; `README.md` states the range and both
+  query-string rules.
 - 2026-09-28 unknown `PIXA_` environment variables abort startup (closes
   #133): a variable whose name starts with `PIXA_` but is neither a
   setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
@@ -224,10 +257,9 @@ exhaustion
 
 # Future Steps
 
-- P1: strip EXIF and other metadata from processed images (privacy)
 - P2: security
     - referer blacklist
-    - per-IP rate limiting
+    - per-IP rate limiting on the image routes
     - per-origin rate limiting
 - P2: HTTP response handling
     - Last-Modified headers

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/getsentry/sentry-go v0.40.0
 	github.com/go-chi/chi/v5 v5.2.3
 	github.com/go-chi/cors v1.2.2
+	github.com/go-chi/httprate v0.16.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/securecookie v1.1.2
 	github.com/prometheus/client_golang v1.23.2
@@ -91,6 +92,7 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/klauspost/cpuid/v2 v2.2.10 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
@@ -113,6 +115,7 @@ require (
 	github.com/tidwall/match v1.1.1 // indirect
 	github.com/tidwall/pretty v1.2.0 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
+	github.com/zeebo/xxh3 v1.0.2 // indirect
 	go.etcd.io/etcd/api/v3 v3.6.2 // indirect
 	go.etcd.io/etcd/client/pkg/v3 v3.6.2 // indirect
 	go.etcd.io/etcd/client/v3 v3.6.2 // indirect

go.sum

@@ -114,6 +114,8 @@ github.com/go-chi/chi/v5 v5.2.3 h1:WQIt9uxdsAbgIYgid+BpYc+liqQZGMHRaUwp0JUcvdE=
 github.com/go-chi/chi/v5 v5.2.3/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops=
 github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE=
 github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
+github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8=
+github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
 github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
 github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=
@@ -251,6 +253,8 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
 github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
+github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE=
+github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
@@ -396,6 +400,10 @@ github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcY
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
+github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
+github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
+github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
+github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA=
 go.etcd.io/etcd/api/v3 v3.6.2 h1:25aCkIMjUmiiOtnBIp6PhNj4KdcURuBak0hU2P1fgRc=
 go.etcd.io/etcd/api/v3 v3.6.2/go.mod h1:eFhhvfR8Px1P6SEuLT600v+vrhdDTdcfMzmnxVXXSbk=
 go.etcd.io/etcd/client/pkg/v3 v3.6.2 h1:zw+HRghi/G8fKpgKdOcEKpnBTE4OO39T6MegA0RopVU=

internal/handlers/auth.go

@@ -18,9 +18,9 @@ import (
 	"sneak.berlin/go/pixa/internal/templates"
 )
 
-// errInvalidFormField reports a generator form field, or the q parameter of
-// /v1/image/, whose value is non-numeric or out of range. The offending field
-// name is wrapped in so the response can name it.
+// errInvalidFormField reports a generator form field, or the q or exp
+// parameter of /v1/image/, whose value is non-numeric or out of range. The
+// offending field name is wrapped in so the response can name it.
 var errInvalidFormField = errors.New("invalid")
 
 // Bounds for the generator's quality and ttl fields; the quality bounds also

internal/handlers/handlers_internal_test.go

@@ -293,12 +293,13 @@ func TestHandleImage_InvalidFitMode_Returns400(t *testing.T) {
 	}
 }
 
-// TestHandleImage_InvalidQuality_Returns400 verifies that the plain image
-// route answers a q that is not a whole number from 1 to 100, an empty one
-// included, with 400 naming q and the value, and a query string that cannot
-// be decoded with 400 showing it, instead of serving the image at the default
-// quality 85.
-func TestHandleImage_InvalidQuality_Returns400(t *testing.T) {
+// TestHandleImage_InvalidQuery_Returns400 verifies that the plain image route
+// answers a q that is not a whole number from 1 to 100, an empty one
+// included, with 400 naming q and the value, a parameter given more than once
+// with 400 naming it, and a query string that cannot be decoded with 400
+// showing it, instead of serving the image at the default quality 85 or at
+// the first value given.
+func TestHandleImage_InvalidQuery_Returns400(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -308,7 +309,9 @@ func TestHandleImage_InvalidQuality_Returns400(t *testing.T) {
 		{"q=0", `invalid q: must be from 1 to 100, got "0"`},
 		{"q=101", `invalid q: must be from 1 to 100, got "101"`},
 		{"q=", `invalid q: not a number, got ""`},
-		{"q=&q=500", `invalid q: not a number, got ""`},
+		{"q=80&q=500", `invalid q: given more than once`},
+		{"q=80&q=", `invalid q: given more than once`},
+		{"fit=cover&fit=contain", `invalid fit: given more than once`},
 		{"q=80%", `invalid query string "q=80%": invalid URL escape "%"`},
 		{
 			"q=50;fit=contain",
@@ -353,3 +356,41 @@ func TestHandleImage_InvalidQuality_Returns400(t *testing.T) {
 		})
 	}
 }
+
+// TestHandleImage_EmptyFit_Returns400 verifies that the plain image route
+// answers a fit that is in the URL but empty with 400 naming fit, instead of
+// serving the image as cover. Only a fit missing from the URL means cover.
+func TestHandleImage_EmptyFit_Returns400(t *testing.T) {
+	t.Parallel()
+
+	fix := setupTestHandler(t)
+
+	r := chi.NewRouter()
+	r.Get("/v1/image/*", fix.handler.HandleImage())
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
+		"/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg?fit=", nil)
+	rec := httptest.NewRecorder()
+
+	r.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+
+	t.Logf("GET %s: %d %s", req.URL, rec.Code, rec.Body)
+
+	var body struct {
+		Error string `json:"error"`
+	}
+
+	err := json.NewDecoder(rec.Body).Decode(&body)
+	if err != nil {
+		t.Fatalf("decoding response body: %v", err)
+	}
+
+	wantError := `invalid fit: not a fit mode, got ""`
+	if body.Error != wantError {
+		t.Errorf("error = %q, want %q", body.Error, wantError)
+	}
+}

internal/handlers/image.go

@@ -94,7 +94,9 @@ func (s *Handlers) parseImageRequest(
 
 	// Parse signature params from query string. r.URL.Query() would silently
 	// drop a pair it cannot decode, such as q=80%, so that q would be served
-	// at 85; a query string that cannot be decoded is refused instead.
+	// at 85; a query string that cannot be decoded is refused instead. A
+	// parameter given more than once is refused too, as only its first value
+	// would be read.
 	query, err := url.ParseQuery(r.URL.RawQuery)
 	if err != nil {
 		s.respondError(w, fmt.Sprintf("invalid query string %q: %v",
@@ -103,13 +105,22 @@ func (s *Handlers) parseImageRequest(
 		return nil, false
 	}
 
+	for name, values := range query {
+		if len(values) > 1 {
+			s.respondError(w, fmt.Sprintf("invalid %s: given more than once",
+				name), http.StatusBadRequest)
+
+			return nil, false
+		}
+	}
+
 	req.Signature = query.Get("sig")
 
-	if expStr := query.Get("exp"); expStr != "" {
-		exp, parseErr := strconv.ParseInt(expStr, 10, 64)
-		if parseErr == nil {
-			req.Expires = time.Unix(exp, 0)
-		}
+	req.Expires, err = parseExpires(query)
+	if err != nil {
+		s.respondError(w, err.Error(), http.StatusBadRequest)
+
+		return nil, false
 	}
 
 	// Parse optional quality and fit params. Only a q missing from the URL is
@@ -133,8 +144,14 @@ func (s *Handlers) parseImageRequest(
 		return nil, false
 	}
 
-	if fit := query.Get("fit"); fit != "" {
-		req.FitMode = imgcache.FitMode(fit)
+	// Only a fit missing from the URL is cover. A fit in the URL that is not a
+	// fit mode is refused by the fit-mode check below; that check would take an
+	// empty fit as missing, so an empty one is refused here.
+	req.FitMode = imgcache.FitMode(query.Get("fit"))
+	if query.Has("fit") && req.FitMode == "" {
+		s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
+
+		return nil, false
 	}
 
 	// Default fit mode if not set
@@ -157,6 +174,26 @@ func (s *Handlers) parseImageRequest(
 	return req, true
 }
 
+// parseExpires reads the exp query parameter, a Unix time in seconds. An exp
+// missing from the URL gives the zero time, which the signature check takes
+// as no expiration. An exp in the URL that is not a whole number, an empty
+// one included, is an error naming exp and the value.
+func parseExpires(query url.Values) (time.Time, error) {
+	if !query.Has("exp") {
+		return time.Time{}, nil
+	}
+
+	expStr := query.Get("exp")
+
+	exp, err := strconv.ParseInt(expStr, 10, 64)
+	if err != nil {
+		return time.Time{}, fmt.Errorf("%w exp: not a number, got %q",
+			errInvalidFormField, expStr)
+	}
+
+	return time.Unix(exp, 0), nil
+}
+
 // respondImageError maps image retrieval errors to HTTP responses.
 func (s *Handlers) respondImageError(
 	w http.ResponseWriter, req *imgcache.ImageRequest, err error,

internal/handlers/image_signature_internal_test.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -118,3 +119,53 @@ func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) {
 		})
 	}
 }
+
+// TestHandleImage_InvalidExp_Returns400 sends a signed-host URL whose exp is
+// not a whole number, and one whose exp is empty. Each is refused with 400
+// naming exp and the value, not with the 401 a URL without exp still gets.
+func TestHandleImage_InvalidExp_Returns400(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		query      string
+		wantStatus int
+		wantError  string
+	}{
+		{"sig=x&exp=banana", http.StatusBadRequest,
+			`invalid exp: not a number, got "banana"`},
+		{"sig=x&exp=", http.StatusBadRequest, `invalid exp: not a number, got ""`},
+		{"sig=x", http.StatusUnauthorized, "unauthorized"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.query, func(t *testing.T) {
+			t.Parallel()
+
+			fix := setupTestHandler(t)
+
+			r := chi.NewRouter()
+			r.Get("/v1/image/*", fix.handler.HandleImage())
+
+			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
+				"/v1/image/"+signedHost+"/images/photo.jpg/50x50.jpeg?"+tt.query, nil)
+			rec := httptest.NewRecorder()
+
+			r.ServeHTTP(rec, req)
+			t.Logf("GET %s: %d %s", req.URL, rec.Code, rec.Body)
+
+			var body struct {
+				Error string `json:"error"`
+			}
+
+			err := json.NewDecoder(rec.Body).Decode(&body)
+			if err != nil {
+				t.Fatalf("decoding response body: %v", err)
+			}
+
+			if rec.Code != tt.wantStatus || body.Error != tt.wantError {
+				t.Errorf("got %d %q, want %d %q",
+					rec.Code, body.Error, tt.wantStatus, tt.wantError)
+			}
+		})
+	}
+}

internal/imageprocessor/imageprocessor.go

@@ -161,6 +161,13 @@ func (p *ImageProcessor) Process(
 	}
 	defer img.Close()
 
+	// Turn the image upright now: encode strips the EXIF orientation tag,
+	// and sizes below must be worked out on the upright image.
+	err = img.AutoRotate()
+	if err != nil {
+		return nil, fmt.Errorf("failed to auto-rotate: %w", err)
+	}
+
 	// Get original dimensions
 	origWidth := img.Width()
 	origHeight := img.Height()
@@ -404,6 +411,21 @@ func (p *ImageProcessor) encode(
 		return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format)
 	}
 
+	// Stripping drops the ICC profile as well, and clients show an image
+	// with no profile as sRGB, so convert to sRGB first. "srgb" names
+	// libvips' built-in profile; govips' own sRGB path variable is set on
+	// first use but read without a lock, so concurrent requests race on it.
+	if img.HasICCProfile() {
+		err := img.TransformICCProfileWithFallback("srgb", "srgb")
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
+		}
+	}
+
+	// Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for
+	// GIF, which carries none of them.
+	params.StripMetadata = true
+
 	output, _, err := img.Export(&params)
 	if err != nil {
 		return nil, err

internal/imageprocessor/imageprocessor_internal_test.go

@@ -9,7 +9,9 @@ import (
 	"image/jpeg"
 	"image/png"
 	"io"
+	"math"
 	"os"
+	"slices"
 	"testing"
 
 	"github.com/davidbyttow/govips/v2/vips"
@@ -561,3 +563,152 @@ func TestImageProcessor_EncodeAVIF(t *testing.T) {
 
 	encodeAndCheck(t, FormatAVIF, 85, mimeAVIF)
 }
+
+// processAndDecode runs input through Process and decodes the output with
+// vips, so a test can inspect the image a client would receive.
+func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef {
+	t.Helper()
+
+	result, err := New(Params{}).Process(
+		context.Background(), bytes.NewReader(input), req,
+	)
+	if err != nil {
+		t.Fatalf("Process() error = %v", err)
+	}
+
+	defer func() { _ = result.Content.Close() }()
+
+	data, err := io.ReadAll(result.Content)
+	if err != nil {
+		t.Fatalf("failed to read result: %v", err)
+	}
+
+	output, err := vips.NewImageFromBuffer(data)
+	if err != nil {
+		t.Fatalf("failed to decode output: %v", err)
+	}
+
+	t.Cleanup(output.Close)
+
+	return output
+}
+
+func TestImageProcessor_StripsEXIF(t *testing.T) {
+	t.Parallel()
+
+	// gps-exif.jpg carries GPS coordinates, a camera make, model and serial
+	// number, and a capture time.
+	input, err := os.ReadFile("testdata/gps-exif.jpg")
+	if err != nil {
+		t.Fatalf("failed to read test JPEG: %v", err)
+	}
+
+	fixture, err := vips.NewImageFromBuffer(input)
+	if err != nil {
+		t.Fatalf("failed to decode test JPEG: %v", err)
+	}
+
+	t.Cleanup(fixture.Close)
+
+	if !slices.Contains(fixture.GetFields(), "exif-ifd3-GPSLatitude") {
+		t.Fatal("testdata/gps-exif.jpg has no GPS latitude")
+	}
+
+	formats := []Format{
+		FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal,
+	}
+
+	for _, format := range formats {
+		t.Run(string(format), func(t *testing.T) {
+			t.Parallel()
+
+			output := processAndDecode(t, input, &Request{Format: format})
+
+			if output.HasExif() {
+				t.Errorf("output has EXIF: %v", output.GetExif())
+			}
+		})
+	}
+}
+
+func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) {
+	t.Parallel()
+
+	// orientation-6.jpg is stored 16x8, red on the left and blue on the
+	// right, with EXIF orientation 6 (turn 90 degrees clockwise to view).
+	// Upright it is 8x16, red on top and blue below.
+	input, err := os.ReadFile("testdata/orientation-6.jpg")
+	if err != nil {
+		t.Fatalf("failed to read test JPEG: %v", err)
+	}
+
+	tests := []struct {
+		name  string
+		size  Size
+		wantW int
+		wantH int
+	}{
+		{name: "original size", size: Size{}, wantW: 8, wantH: 16},
+		{name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			output := processAndDecode(t, input, &Request{
+				Size:   tt.size,
+				Format: FormatPNG,
+			})
+
+			if output.Width() != tt.wantW || output.Height() != tt.wantH {
+				t.Fatalf("output is %dx%d, want %dx%d",
+					output.Width(), output.Height(), tt.wantW, tt.wantH)
+			}
+
+			top, err := output.GetPoint(tt.wantW/2, 0)
+			if err != nil {
+				t.Fatalf("GetPoint() error = %v", err)
+			}
+
+			bottom, err := output.GetPoint(tt.wantW/2, tt.wantH-1)
+			if err != nil {
+				t.Fatalf("GetPoint() error = %v", err)
+			}
+
+			if top[0] <= top[2] || bottom[2] <= bottom[0] {
+				t.Errorf("top pixel = %v, bottom pixel = %v, want red above blue",
+					top, bottom)
+			}
+		})
+	}
+}
+
+func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) {
+	t.Parallel()
+
+	// display-p3.jpg is a flat 8x8 image with the Display P3 profile
+	// embedded, filled with Display P3 (234, 51, 35), which is sRGB red.
+	input, err := os.ReadFile("testdata/display-p3.jpg")
+	if err != nil {
+		t.Fatalf("failed to read test JPEG: %v", err)
+	}
+
+	output := processAndDecode(t, input, &Request{Format: FormatPNG})
+
+	if output.HasICCProfile() {
+		t.Error("output has an ICC profile")
+	}
+
+	pixel, err := output.GetPoint(4, 4)
+	if err != nil {
+		t.Fatalf("GetPoint() error = %v", err)
+	}
+
+	want := []float64{255, 0, 0}
+	for i := range want {
+		if math.Abs(pixel[i]-want[i]) > 5 {
+			t.Fatalf("pixel = %v, want within 5 of %v", pixel, want)
+		}
+	}
+}

internal/imgcache/cache.go

@@ -125,7 +125,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 	}
 
 	variants, err := NewVariantStorage(
-		filepath.Join(config.StateDir, "cache", "variants"),
+		filepath.Join(config.StateDir, "cache", "variants"), log,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage: %w", err)
@@ -263,23 +263,7 @@ func (c *Cache) StoreSource(
 		return "", fmt.Errorf("failed to insert source metadata: %w", err)
 	}
 
-	// Store metadata JSON file
-	meta := &SourceMetadata{
-		Host:            req.SourceHost,
-		Path:            req.SourcePath,
-		Query:           req.SourceQuery,
-		ContentHash:     string(contentHash),
-		StatusCode:      result.StatusCode,
-		ContentType:     result.ContentType,
-		ContentLength:   result.ContentLength,
-		ResponseHeaders: result.Headers,
-		FetchedAt:       time.Now().UTC().Unix(),
-		FetchDurationMs: result.FetchDurationMs,
-		RemoteAddr:      result.RemoteAddr,
-	}
-
-	// A failure here is non-fatal; the metadata is in the database.
-	_ = c.srcMetadata.Store(req.SourceHost, pathHash, meta)
+	c.writeMetadataSidecar(req, pathHash, contentHash, result)
 
 	c.notifyWritePressure()
 
@@ -436,12 +420,19 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 	}
 
 	// Get actual item count and total size from content tables
-	_ = c.db.QueryRowContext(ctx,
+	err = c.db.QueryRowContext(ctx,
 		`SELECT COUNT(*) FROM request_cache`,
 	).Scan(&stats.TotalItems)
-	_ = c.db.QueryRowContext(ctx,
+	if err != nil {
+		c.log.Warn("failed to count cache items for stats", "error", err)
+	}
+
+	err = c.db.QueryRowContext(ctx,
 		`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`,
 	).Scan(&stats.TotalSizeBytes)
+	if err != nil {
+		c.log.Warn("failed to sum cache size for stats", "error", err)
+	}
 
 	// Compute hit rate as a ratio
 	if stats.HitCount+stats.MissCount > 0 {
@@ -453,15 +444,17 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 
 // IncrementStats increments cache statistics.
 func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
+	var err error
+
 	if hit {
-		_, _ = c.db.ExecContext(ctx, `
+		_, err = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET hit_count = hit_count + 1,
 			    last_updated_at = CURRENT_TIMESTAMP
 			WHERE id = 1
 		`)
 	} else {
-		_, _ = c.db.ExecContext(ctx, `
+		_, err = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET miss_count = miss_count + 1,
 			    last_updated_at = CURRENT_TIMESTAMP
@@ -469,14 +462,52 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
 		`)
 	}
 
+	if err != nil {
+		c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err)
+	}
+
 	if fetchBytes > 0 {
-		_, _ = c.db.ExecContext(ctx, `
+		_, err = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET upstream_fetch_count = upstream_fetch_count + 1,
 			    upstream_fetch_bytes = upstream_fetch_bytes + ?,
 			    last_updated_at = CURRENT_TIMESTAMP
 			WHERE id = 1
 		`, fetchBytes)
+		if err != nil {
+			c.log.Warn("failed to count upstream fetch",
+				"fetch_bytes", fetchBytes, "error", err)
+		}
+	}
+}
+
+// writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
+// A failure is logged and is otherwise non-fatal; the metadata is in the
+// database.
+func (c *Cache) writeMetadataSidecar(
+	req *ImageRequest,
+	pathHash PathHash,
+	contentHash ContentHash,
+	result *httpfetcher.FetchResult,
+) {
+	meta := &SourceMetadata{
+		Host:            req.SourceHost,
+		Path:            req.SourcePath,
+		Query:           req.SourceQuery,
+		ContentHash:     string(contentHash),
+		StatusCode:      result.StatusCode,
+		ContentType:     result.ContentType,
+		ContentLength:   result.ContentLength,
+		ResponseHeaders: result.Headers,
+		FetchedAt:       time.Now().UTC().Unix(),
+		FetchDurationMs: result.FetchDurationMs,
+		RemoteAddr:      result.RemoteAddr,
+	}
+
+	err := c.srcMetadata.Store(req.SourceHost, pathHash, meta)
+	if err != nil {
+		c.log.Warn("failed to write metadata sidecar",
+			"host", req.SourceHost, "path_hash", pathHash, "error", err)
 	}
 }
 
@@ -528,10 +559,14 @@ func (c *Cache) checkNegativeCache(
 	// Check if expired
 	if time.Now().After(expiresAt) {
 		// Clean up expired entry
-		_, _ = c.db.ExecContext(ctx, `
+		_, err = c.db.ExecContext(ctx, `
 			DELETE FROM negative_cache
 			WHERE source_host = ? AND source_path = ? AND source_query = ?
 		`, req.SourceHost, req.SourcePath, req.SourceQuery)
+		if err != nil {
+			c.log.Warn("failed to delete expired negative cache entry",
+				"host", req.SourceHost, "path", req.SourcePath, "error", err)
+		}
 
 		return false, nil
 	}

internal/imgcache/service.go

@@ -322,7 +322,12 @@ func (s *Service) fetchAndProcess(
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {
 			statusCode := extractStatusCode(err)
-			_ = s.cache.StoreNegative(ctx, req, statusCode, 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)
+			}
 		}
 
 		return nil, fmt.Errorf("upstream fetch failed: %w", err)

internal/imgcache/stats_internal_test.go

@@ -1,9 +1,12 @@
 package imgcache
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
+	"log/slog"
 	"math"
+	"strings"
 	"testing"
 	"time"
 
@@ -101,3 +104,82 @@ func TestStats_ZeroCounts(t *testing.T) {
 		t.Errorf("HitRate = %f, want 0.0 for zero counts", stats.HitRate)
 	}
 }
+
+// TestStats_LogsFailedCountQueries verifies that a failed item count query
+// and a failed size query are each logged at warn and Stats still succeeds.
+func TestStats_LogsFailedCountQueries(t *testing.T) {
+	t.Parallel()
+
+	db := setupStatsTestDB(t)
+
+	var logBuf bytes.Buffer
+
+	cache, err := NewCache(db, CacheConfig{
+		StateDir:    t.TempDir(),
+		CacheTTL:    time.Hour,
+		NegativeTTL: 5 * time.Minute,
+		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = db.ExecContext(t.Context(),
+		`DROP TABLE request_cache; DROP TABLE output_content`)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = cache.Stats(t.Context())
+	if err != nil {
+		t.Fatalf("Stats() error = %v, want nil", err)
+	}
+
+	for _, msg := range []string{
+		"failed to count cache items for stats",
+		"failed to sum cache size for stats",
+	} {
+		want := `"level":"WARN","msg":"` + msg + `"`
+		if !strings.Contains(logBuf.String(), want) {
+			t.Errorf("log missing %s; got %q", want, logBuf.String())
+		}
+	}
+}
+
+// TestIncrementStats_LogsFailedUpdates verifies that a failed hit or miss
+// count update and a failed upstream fetch count update are each logged at
+// warn.
+func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
+	t.Parallel()
+
+	db := setupStatsTestDB(t)
+
+	var logBuf bytes.Buffer
+
+	cache, err := NewCache(db, CacheConfig{
+		StateDir:    t.TempDir(),
+		CacheTTL:    time.Hour,
+		NegativeTTL: 5 * time.Minute,
+		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = db.ExecContext(t.Context(), `DROP TABLE cache_stats`)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cache.IncrementStats(t.Context(), false, 1024)
+
+	for _, msg := range []string{
+		"failed to count cache hit or miss",
+		"failed to count upstream fetch",
+	} {
+		want := `"level":"WARN","msg":"` + msg + `"`
+		if !strings.Contains(logBuf.String(), want) {
+			t.Errorf("log missing %s; got %q", want, logBuf.String())
+		}
+	}
+}

internal/imgcache/storage.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"time"
@@ -392,6 +393,7 @@ func CacheKey(req *ImageRequest) VariantKey {
 // Unlike ContentStorage, the key is provided by the caller (not computed from content).
 type VariantStorage struct {
 	baseDir string
+	log     *slog.Logger
 }
 
 // VariantMeta contains metadata about a cached variant.
@@ -404,13 +406,14 @@ type VariantMeta struct {
 }
 
 // NewVariantStorage creates a new variant storage at the given base directory.
-func NewVariantStorage(baseDir string) (*VariantStorage, error) {
+// A failed .meta write is logged to log.
+func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error) {
 	err := os.MkdirAll(baseDir, StorageDirPerm)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage directory: %w", err)
 	}
 
-	return &VariantStorage{baseDir: baseDir}, nil
+	return &VariantStorage{baseDir: baseDir, log: log}, nil
 }
 
 // Store writes content and metadata to storage at the given key.
@@ -478,7 +481,11 @@ func (s *VariantStorage) Store(
 	}
 
 	// Metadata write failure is non-fatal; content is already stored.
-	_ = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	err = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	if err != nil {
+		s.log.Warn("failed to write variant metadata sidecar",
+			"path", metaPath, "error", err)
+	}
 
 	return size, nil
 }

internal/imgcache/storage_internal_test.go

@@ -4,8 +4,10 @@ import (
 	"bytes"
 	"errors"
 	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -404,3 +406,35 @@ func TestCacheKey(t *testing.T) {
 		t.Error("CacheKey() produced same key for different quality")
 	}
 }
+
+// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
+// that fails is logged at warn and the store still succeeds.
+func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
+	t.Parallel()
+
+	var logBuf bytes.Buffer
+
+	storage, err := NewVariantStorage(
+		t.TempDir(), slog.New(slog.NewJSONHandler(&logBuf, nil)))
+	if err != nil {
+		t.Fatalf("NewVariantStorage() error = %v", err)
+	}
+
+	key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat})
+
+	// A directory where the .meta file goes makes the .meta write fail.
+	err = os.MkdirAll(storage.keyToPath(key)+".meta", StorageDirPerm)
+	if err != nil {
+		t.Fatalf("failed to create directory: %v", err)
+	}
+
+	_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
+	if err != nil {
+		t.Fatalf("Store() error = %v, want nil", err)
+	}
+
+	want := `"level":"WARN","msg":"failed to write variant metadata sidecar"`
+	if !strings.Contains(logBuf.String(), want) {
+		t.Errorf("log missing %s; got %q", want, logBuf.String())
+	}
+}

internal/middleware/middleware.go

@@ -4,11 +4,13 @@ package middleware
 import (
 	"log/slog"
 	"net/http"
+	"net/netip"
 	"time"
 
 	basicauth "github.com/99designs/basicauth-go"
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/go-chi/cors"
+	"github.com/go-chi/httprate"
 	metrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
@@ -88,6 +90,31 @@ func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
 	}
 }
 
+// RateLimit returns a middleware that limits each client to requestLimit
+// requests per window and refuses a request over the limit with 429 Too Many
+// Requests and a Retry-After header. Clients are told apart by the address
+// the ClientIP middleware stored in the request context, so ClientIP must
+// run first. An IPv6 client is counted by its /64, which one client usually
+// holds whole; an IPv4-mapped address (::ffff:a.b.c.d) is counted as the
+// IPv4 address it carries, since every such address falls in the same /64.
+// Counts are kept only for the current and the previous window, so memory
+// stays bounded.
+func (s *Middleware) RateLimit(
+	requestLimit int, window time.Duration,
+) func(http.Handler) http.Handler {
+	return httprate.LimitBy(requestLimit, window,
+		func(r *http.Request) (string, error) {
+			ip := clientip.FromContext(r.Context())
+
+			addr, err := netip.ParseAddr(ip)
+			if err == nil {
+				ip = addr.Unmap().String()
+			}
+
+			return httprate.CanonicalizeIP(ip), nil
+		})
+}
+
 type loggingResponseWriter struct {
 	http.ResponseWriter
 

internal/server/login_rate_limit_internal_test.go

@@ -0,0 +1,280 @@
+package server
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"net/url"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"strings"
+	"testing"
+
+	"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/handlers"
+	"sneak.berlin/go/pixa/internal/logger"
+	"sneak.berlin/go/pixa/internal/middleware"
+)
+
+// testSigningKey is a throwaway signing key; submitting it logs in.
+const testSigningKey = "test-signing-key-0123456789abcdef"
+
+// wrongKey is submitted for a failed login.
+const wrongKey = "not-the-signing-key"
+
+// Addresses for the login rate limit tests. The test server trusts
+// 10.0.0.0/8 as its proxies, so the X-Forwarded-For sent by proxyPeer is
+// believed and the one sent by firstClient or secondClient is ignored.
+const (
+	firstClient     = "198.51.100.1:40000"
+	secondClient    = "198.51.100.2:40000"
+	proxyPeer       = "10.0.0.1:40000"
+	firstForwarded  = "203.0.113.1"
+	secondForwarded = "203.0.113.2"
+)
+
+// csrfFieldPattern extracts the CSRF token rendered into the login form.
+var csrfFieldPattern = regexp.MustCompile(
+	`name="gorilla\.csrf\.Token" value="([^"]+)"`)
+
+// newTestServer builds the server's real routes from the constructors
+// cmd/pixad uses, with a throwaway state directory. Debug marks requests
+// as plain HTTP, so the CSRF check runs without an https Referer.
+func newTestServer(t *testing.T) *Server {
+	t.Helper()
+
+	stateDir := t.TempDir()
+	cfg := &config.Config{
+		Debug:          true,
+		SigningKey:     testSigningKey,
+		StateDir:       stateDir,
+		DBURL:          "file:" + filepath.Join(stateDir, "state.sqlite3"),
+		TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
+	}
+
+	lc := fxtest.NewLifecycle(t)
+
+	log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
+	if err != nil {
+		t.Fatalf("logger.New() error = %v", err)
+	}
+
+	db, err := database.New(lc, database.Params{Logger: log, Config: cfg})
+	if err != nil {
+		t.Fatalf("database.New() error = %v", err)
+	}
+
+	h, err := handlers.New(lc, handlers.Params{
+		Logger: log, Database: db, Config: cfg,
+	})
+	if err != nil {
+		t.Fatalf("handlers.New() error = %v", err)
+	}
+
+	mw, err := middleware.New(lc, middleware.Params{Logger: log, Config: cfg})
+	if err != nil {
+		t.Fatalf("middleware.New() error = %v", err)
+	}
+
+	lc.RequireStart()
+	t.Cleanup(lc.RequireStop)
+
+	s := &Server{config: cfg, mw: mw, h: h}
+	s.SetupRoutes()
+
+	return s
+}
+
+// clientRequest builds a request for / arriving from remoteAddr, carrying
+// forwardedFor as its X-Forwarded-For header when that is not empty.
+func clientRequest(
+	t *testing.T, method string, body io.Reader, remoteAddr, forwardedFor string,
+) *http.Request {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), method, "/", body)
+	req.RemoteAddr = remoteAddr
+
+	if forwardedFor != "" {
+		req.Header.Set("X-Forwarded-For", forwardedFor)
+	}
+
+	return req
+}
+
+// postLogin loads the login form with GET / and submits key in it with
+// POST /, as a browser does, both from the same client. GET / is not rate
+// limited, so the form must load even for a client over the limit.
+func postLogin(
+	t *testing.T, s *Server, remoteAddr, forwardedFor, key string,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	page := httptest.NewRecorder()
+	s.ServeHTTP(page,
+		clientRequest(t, http.MethodGet, nil, remoteAddr, forwardedFor))
+
+	if page.Code != http.StatusOK {
+		t.Fatalf("GET / status = %d, want %d", page.Code, http.StatusOK)
+	}
+
+	match := csrfFieldPattern.FindStringSubmatch(page.Body.String())
+	if match == nil {
+		t.Fatalf("no CSRF token field found in the login form")
+	}
+
+	form := url.Values{"key": {key}, "gorilla.csrf.Token": {match[1]}}
+	req := clientRequest(t, http.MethodPost,
+		strings.NewReader(form.Encode()), remoteAddr, forwardedFor)
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	for _, c := range page.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// tripLoginRateLimit makes LoginAttemptsPerMinute failed logins from one
+// client, each answered with the login form again, then one more, which
+// must be refused with 429. It returns the response to that last attempt.
+func tripLoginRateLimit(
+	t *testing.T, s *Server, remoteAddr, forwardedFor string,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	for attempt := range LoginAttemptsPerMinute {
+		rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("failed login %d status = %d, want %d",
+				attempt+1, rec.Code, http.StatusOK)
+		}
+	}
+
+	rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
+	if rec.Code != http.StatusTooManyRequests {
+		t.Fatalf("login over the limit status = %d, want %d",
+			rec.Code, http.StatusTooManyRequests)
+	}
+
+	return rec
+}
+
+// TestLoginRateLimitRefusesAttemptOverLimit verifies the login attempt
+// after LoginAttemptsPerMinute failed ones from one client is refused with
+// 429 and a Retry-After header, and that the client cannot get around the
+// limit by sending X-Forwarded-For: from a peer that is not a trusted
+// proxy, the header is ignored.
+func TestLoginRateLimitRefusesAttemptOverLimit(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	rec := tripLoginRateLimit(t, s, firstClient, "")
+
+	retryAfter := rec.Header().Get("Retry-After")
+
+	seconds, err := strconv.Atoi(retryAfter)
+	if err != nil || seconds <= 0 {
+		t.Errorf("Retry-After = %q, want a positive number of seconds",
+			retryAfter)
+	}
+
+	rec = postLogin(t, s, firstClient, secondForwarded, wrongKey)
+	if rec.Code != http.StatusTooManyRequests {
+		t.Errorf("login with X-Forwarded-For from an untrusted peer "+
+			"status = %d, want %d", rec.Code, http.StatusTooManyRequests)
+	}
+}
+
+// TestLoginRateLimitLeavesOtherClientsAlone verifies one client going over
+// the limit does not limit another: a failed login from a different
+// address is answered with the login form, and the signing key still logs
+// it in.
+func TestLoginRateLimitLeavesOtherClientsAlone(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	tripLoginRateLimit(t, s, firstClient, "")
+
+	rec := postLogin(t, s, secondClient, "", wrongKey)
+	if rec.Code != http.StatusOK {
+		t.Errorf("failed login from another client status = %d, want %d",
+			rec.Code, http.StatusOK)
+	}
+
+	rec = postLogin(t, s, secondClient, "", testSigningKey)
+	if rec.Code != http.StatusSeeOther {
+		t.Errorf("login with the signing key from another client "+
+			"status = %d, want %d", rec.Code, http.StatusSeeOther)
+	}
+}
+
+// TestLoginRateLimitCountsClientsBehindProxySeparately verifies the limit
+// counts the client address resolved from X-Forwarded-For, not the address
+// of the trusted proxy the requests arrive from, so two clients behind the
+// same proxy are counted separately.
+func TestLoginRateLimitCountsClientsBehindProxySeparately(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	tripLoginRateLimit(t, s, proxyPeer, firstForwarded)
+
+	rec := postLogin(t, s, proxyPeer, secondForwarded, wrongKey)
+	if rec.Code != http.StatusOK {
+		t.Errorf("failed login from a second client behind the proxy "+
+			"status = %d, want %d", rec.Code, http.StatusOK)
+	}
+}
+
+// TestLoginRateLimitCountsIPv6ClientsByPrefix verifies an IPv6 client is
+// counted by its /64: another address in the same /64 is refused too,
+// while an address in a different /64 is not.
+func TestLoginRateLimitCountsIPv6ClientsByPrefix(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	tripLoginRateLimit(t, s, proxyPeer, "2001:db8::1")
+
+	rec := postLogin(t, s, proxyPeer, "2001:db8::2", wrongKey)
+	if rec.Code != http.StatusTooManyRequests {
+		t.Errorf("login from the same /64 status = %d, want %d",
+			rec.Code, http.StatusTooManyRequests)
+	}
+
+	rec = postLogin(t, s, proxyPeer, "2001:db8:0:1::1", wrongKey)
+	if rec.Code != http.StatusOK {
+		t.Errorf("login from another /64 status = %d, want %d",
+			rec.Code, http.StatusOK)
+	}
+}
+
+// TestLoginRateLimitCountsIPv4MappedClientsSeparately verifies an IPv4
+// client that the proxy forwards in IPv4-mapped IPv6 form (::ffff:a.b.c.d)
+// is counted by its IPv4 address, not by the /64 that every such address
+// shares, so two of them behind the proxy are counted separately.
+func TestLoginRateLimitCountsIPv4MappedClientsSeparately(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	tripLoginRateLimit(t, s, proxyPeer, "::ffff:"+firstForwarded)
+
+	rec := postLogin(t, s, proxyPeer, "::ffff:"+secondForwarded, wrongKey)
+	if rec.Code != http.StatusOK {
+		t.Errorf("failed login from a second IPv4-mapped client "+
+			"status = %d, want %d", rec.Code, http.StatusOK)
+	}
+}

internal/server/routes.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"net/http"
+	"time"
 
 	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/go-chi/chi/v5"
@@ -12,6 +13,10 @@ import (
 	"sneak.berlin/go/pixa/internal/static"
 )
 
+// LoginAttemptsPerMinute is how many login attempts (POST /) one client may
+// make per minute; the next is refused with 429 Too Many Requests.
+const LoginAttemptsPerMinute = 5
+
 // SetupRoutes configures all HTTP routes.
 func (s *Server) SetupRoutes() {
 	s.router = chi.NewRouter()
@@ -50,11 +55,14 @@ func (s *Server) SetupRoutes() {
 	// token cookie is independent of the session cookie, so it also
 	// covers the login POST, where no session exists yet. LimitBody caps
 	// the POST body ahead of CSRF, which reads its token from that body.
+	// The login POST is rate limited per client after both, so every
+	// attempt that reaches the signing key comparison is counted.
 	s.router.Group(func(r chi.Router) {
 		r.Use(s.h.LimitBody(handlers.MaxFormBytes))
 		r.Use(s.h.CSRF())
 		r.Get("/", s.h.HandleRoot())
-		r.Post("/", s.h.HandleRoot())
+		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute)).
+			Post("/", s.h.HandleRoot())
 		r.Post("/generate", s.h.HandleGenerateURL())
 	})