From 306e075b5c60a20f189f0f420536534ad0268f2c Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 28 Sep 2026 10:22:53 +0000 Subject: [PATCH] Test that the URL signature covers quality and fit (closes #60) Add Quality and FitMode to signature.Request, not yet part of the signed data, and tests that a signature for quality 85 and fit cover is refused when replayed with another quality or fit: one against Signer.Verify, one through the image route. The route test also sends the same signature with no q or fit and with q=85&fit=cover, which must both pass, and sends URLs built by imgcache.Service.GenerateSignedURL. The replay cases fail until the next commit. Model: opus-4-8 (implementation); opus-5-5 (rework) --- .../handlers/image_signature_internal_test.go | 120 ++++++++++++++++++ internal/signature/quality_fit_test.go | 68 ++++++++++ internal/signature/signature.go | 9 ++ internal/signature/signature_test.go | 2 + 4 files changed, 199 insertions(+) create mode 100644 internal/handlers/image_signature_internal_test.go create mode 100644 internal/signature/quality_fit_test.go diff --git a/internal/handlers/image_signature_internal_test.go b/internal/handlers/image_signature_internal_test.go new file mode 100644 index 0000000..a045f0e --- /dev/null +++ b/internal/handlers/image_signature_internal_test.go @@ -0,0 +1,120 @@ +package handlers + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/signature" +) + +// signedHost is not on the allowlist setupTestHandler builds, so a request +// for it needs a valid signature. No image is served for it: a request that +// passes the signature check gets 502 from the failed fetch, and one that +// fails the check gets 401. +const signedHost = "signed.example.com" + +// getImage sends a GET for target to the image route of fix and returns the +// response status. +func getImage(t *testing.T, fix *testFixtures, target string) int { + t.Helper() + + r := chi.NewRouter() + r.Get("/v1/image/*", fix.handler.HandleImage()) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + t.Logf("GET %s: %d", target, rec.Code) + + return rec.Code +} + +// TestHandleImage_SignatureCoversQualityAndFit signs a URL for quality 85 +// and fit cover, the values the route uses when a URL has no q or fit, and +// sends that signature with each q and fit below. +func TestHandleImage_SignatureCoversQualityAndFit(t *testing.T) { + t.Parallel() + + expires := time.Now().Add(time.Hour) + signer := signature.New("test-signing-key-must-be-32-chars") + sig := signer.Sign(&signature.Request{ + SourceHost: signedHost, + SourcePath: "/images/photo.jpg", + Width: 50, + Height: 50, + Format: string(imgcache.FormatJPEG), + Quality: 85, + FitMode: string(imgcache.FitCover), + Expires: expires, + }) + signedURL := fmt.Sprintf("/v1/image/%s/images/photo.jpg/50x50.jpeg?sig=%s&exp=%d", + signedHost, sig, expires.Unix()) + + tests := []struct { + name string + query string + wantStatus int + }{ + {"no q or fit", "", http.StatusBadGateway}, + {"q=85 and fit=cover", "&q=85&fit=cover", http.StatusBadGateway}, + {"replayed with q=40", "&q=40", http.StatusUnauthorized}, + {"replayed with fit=contain", "&fit=contain", http.StatusUnauthorized}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + status := getImage(t, setupTestHandler(t), signedURL+tt.query) + if status != tt.wantStatus { + t.Errorf("status = %d, want %d", status, tt.wantStatus) + } + }) + } +} + +// TestHandleImage_GeneratedSignedURLVerifies sends URLs built by the +// service's signed-URL generator to the route. +func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + quality int + fitMode imgcache.FitMode + }{ + {"quality 40 and fit contain", 40, imgcache.FitContain}, + {"quality and fit unset", 0, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fix := setupTestHandler(t) + + signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{ + SourceHost: signedHost, + SourcePath: "/images/photo.jpg", + Size: imgcache.Size{Width: 50, Height: 50}, + Format: imgcache.FormatJPEG, + Quality: tt.quality, + FitMode: tt.fitMode, + }, time.Hour) + if err != nil { + t.Fatalf("GenerateSignedURL() error = %v", err) + } + + status := getImage(t, fix, signedURL) + if status != http.StatusBadGateway { + t.Errorf("status = %d, want %d", status, http.StatusBadGateway) + } + }) + } +} diff --git a/internal/signature/quality_fit_test.go b/internal/signature/quality_fit_test.go new file mode 100644 index 0000000..bebbec4 --- /dev/null +++ b/internal/signature/quality_fit_test.go @@ -0,0 +1,68 @@ +package signature_test + +import ( + "errors" + "testing" + "time" + + "sneak.berlin/go/pixa/internal/signature" +) + +// signedQualityFitRequest returns a request signed for quality 85 and fit +// mode "cover", the effective defaults the handler applies before +// verification. +func signedQualityFitRequest(signer *signature.Signer) *signature.Request { + req := &signature.Request{ + SourceHost: testHost, + SourcePath: testPath, + Width: 800, + Height: 600, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + Expires: time.Now().Add(1 * time.Hour), + } + req.Signature = signer.Sign(req) + + return req +} + +// TestSigner_Verify_QualityAndFitAreSigned proves that quality and fit are +// covered by the signature: a URL signed for one quality or fit mode must +// not verify when replayed with a different quality or fit mode. This is the +// amplification vector from the issue — one signed URL replayed across many +// quality and fit values yields many unauthorized cache entries and +// transcodes — so it must be rejected. +func TestSigner_Verify_QualityAndFitAreSigned(t *testing.T) { + t.Parallel() + + signer := signature.New("test-secret-key") + + cases := []struct { + name string + tamper func(r *signature.Request) + }{ + { + name: "replayed with different quality", + tamper: func(r *signature.Request) { r.Quality = 40 }, + }, + { + name: "replayed with different fit mode", + tamper: func(r *signature.Request) { r.FitMode = testFitContain }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + req := signedQualityFitRequest(signer) + tt.tamper(req) + + err := signer.Verify(req) + if !errors.Is(err, signature.ErrInvalid) { + t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid) + } + }) + } +} diff --git a/internal/signature/signature.go b/internal/signature/signature.go index 1a5b00e..f044ffa 100644 --- a/internal/signature/signature.go +++ b/internal/signature/signature.go @@ -37,6 +37,15 @@ type Request struct { Height int // Format is the requested output format (e.g. "webp"). Format string + // Quality is the requested output quality (1-100) for lossy formats. + // It is the effective value the request resolves to: callers pass the + // default quality when the request omits the parameter, so an omitted + // quality signs identically to that same value stated explicitly. + Quality int + // FitMode is how the image is fit into the requested dimensions + // (e.g. "cover"). Like Quality it is the effective value: callers pass + // the default fit mode when the request omits the parameter. + FitMode string // Signature is the HMAC signature to verify. Signature string // Expires is the signature expiration timestamp. diff --git a/internal/signature/signature_test.go b/internal/signature/signature_test.go index e2b663d..1712fca 100644 --- a/internal/signature/signature_test.go +++ b/internal/signature/signature_test.go @@ -15,6 +15,8 @@ const ( testPath = "/photos/cat.jpg" testFormatWebP = "webp" testFormatPNG = "png" + testFitCover = "cover" + testFitContain = "contain" testSignedPath = "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp" testSig = "abc123" )