From db784bf5616c5cb5e09561dede85b7368ec0ede5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 13:02:21 +0200 Subject: [PATCH] Include quality and fit in the URL signature (closes #60) The signed data is now host:path:query:width:height:format:expiration:quality:fit. The route turns a missing q into 85 and a missing fit into cover before checking the signature, so those are the values signed for a URL without them; imgcache fills both from the parsed request. imgcache.Service.GenerateSignedURL now writes q and fit into the URL next to sig and exp, first setting an unset quality or fit to 85 or cover, so a generated URL verifies for the values it signed. The known-answer vectors in golden_test.go, including one for quality 40 and fit contain, and the README signature section describe the new format. Model: opus-4-8 (implementation); opus-5-5 (rework) --- README.md | 23 ++-- TODO.md | 7 + .../handlers/image_signature_internal_test.go | 120 ++++++++++++++++++ internal/imgcache/service.go | 18 ++- internal/signature/golden_test.go | 48 +++++-- internal/signature/quality_fit_test.go | 68 ++++++++++ internal/signature/signature.go | 25 +++- internal/signature/signature_test.go | 2 + 8 files changed, 285 insertions(+), 26 deletions(-) create mode 100644 internal/handlers/image_signature_internal_test.go create mode 100644 internal/signature/quality_fit_test.go diff --git a/README.md b/README.md index 0356e75..b167214 100644 --- a/README.md +++ b/README.md @@ -79,14 +79,14 @@ hosts require an HMAC-SHA256 signature. Signatures use HMAC-SHA256 and include an expiration timestamp to prevent replay attacks. Signatures are **exact match only**: every -component (host, path, query, dimensions, format, expiration) must -match exactly what was signed. No suffix matching, wildcard matching, -or partial matching is supported. +component (host, path, query, dimensions, format, expiration, quality, +fit) must match exactly what was signed. No suffix matching, wildcard +matching, or partial matching is supported. **Signed data format** (colon-separated): ``` -HMAC-SHA256(secret, "host:path:query:width:height:format:expiration") +HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit") ``` Where: @@ -98,18 +98,25 @@ Where: - `height` — requested height in pixels, `0` for original - `format` — output format (jpeg, png, webp, avif, gif, orig) - `expiration` — Unix timestamp when signature expires +- `quality` — the URL's `q` query parameter (1-100), or `85` when the URL + has no `q` +- `fit` — the URL's `fit` query parameter (cover, contain, fill, inside, + outside), or `cover` when the URL has no `fit` -**Example:** resize -`https://cdn.example.com/photos/cat.jpg` to 800x600 WebP with -expiration 1704067200: +**Example:** resize `https://cdn.example.com/photos/cat.jpg` to 800x600 +WebP with expiration 1704067200, default quality and fit: 1. Build input: - `cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200` + `cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200:85:cover` 2. Compute HMAC-SHA256 with your secret key 3. Base64URL-encode the result 4. URL: `/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp?sig=&exp=1704067200` +For the same image at quality 40 with fit `contain`, the input ends in +`:40:contain` and the URL is +`/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp?sig=&exp=1704067200&q=40&fit=contain`. + **Allowlist patterns:** - **Exact match**: `cdn.example.com` — matches only that host diff --git a/TODO.md b/TODO.md index 0c52f04..fb5bcd3 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,13 @@ exhaustion # Completed Steps +- 2026-09-28 quality and fit in the URL signature (closes #60): the signed + data is now `host:path:query:width:height:format:expiration:quality:fit`, + using `85` and `cover` when the URL has no `q` or `fit`, so one signed + URL can no longer be replayed across other quality and fit values to + create unauthorized cache entries and transcodes; the known-answer + vectors in `internal/signature/golden_test.go` and the README signature + specification describe the new format. - 2026-09-28 Docker image healthcheck (closes #111): a `HEALTHCHECK` in the runtime stage probing `/.well-known/healthcheck.json` with busybox `wget`; `script/docker-smoke` (`make docker-smoke`) builds the image, 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/imgcache/service.go b/internal/imgcache/service.go index ecb745d..d6e07df 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -205,12 +205,23 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { return s.signer.Verify(signatureRequest(req)) } -// GenerateSignedURL generates a signed URL for the given request. +// GenerateSignedURL generates a signed URL for the given request. The URL +// carries q and fit next to sig and exp, so the image route verifies it for +// the quality and fit it was signed with. An unset quality or fit is first +// set to 85 or cover, the values the route uses when a URL has no q or fit. func (s *Service) GenerateSignedURL( baseURL string, req *ImageRequest, ttl time.Duration, ) (string, error) { + if req.Quality == 0 { + req.Quality = 85 + } + + if req.FitMode == "" { + req.FitMode = FitCover + } + sigReq := signatureRequest(req) path, sig, exp := s.signer.GenerateSignedURL(sigReq, ttl) @@ -218,7 +229,8 @@ func (s *Service) GenerateSignedURL( req.Expires = sigReq.Expires req.Signature = sigReq.Signature - return fmt.Sprintf("%s%s?sig=%s&exp=%d", baseURL, path, sig, exp), nil + return fmt.Sprintf("%s%s?sig=%s&exp=%d&q=%d&fit=%s", + baseURL, path, sig, exp, req.Quality, req.FitMode), nil } // loadCachedSource attempts to load source content from cache, returning nil @@ -452,6 +464,8 @@ func signatureRequest(req *ImageRequest) *signature.Request { Width: req.Size.Width, Height: req.Size.Height, Format: string(req.Format), + Quality: req.Quality, + FitMode: string(req.FitMode), Signature: req.Signature, Expires: req.Expires, } diff --git a/internal/signature/golden_test.go b/internal/signature/golden_test.go index 26385e0..126b05c 100644 --- a/internal/signature/golden_test.go +++ b/internal/signature/golden_test.go @@ -41,9 +41,12 @@ func goldenVectors() []goldenVector { Width: 800, Height: 600, Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, }, - // Signed data: "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200" - wantSignature: "x5PfPp8QSDo0cJT96od-AEgrQyOVLfqifH5sst61_-w=", + // Signed data: + // "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200:85:cover" + wantSignature: "kdqeGoW2SX7qnaYtoB970wEnLydn0UnIgQYQLfAnjXQ=", wantSignedPath: testSignedPath, }, { @@ -55,10 +58,12 @@ func goldenVectors() []goldenVector { Width: 800, Height: 600, Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, }, // Signed data: - // "cdn.example.com:/photos/cat.jpg:token=abc&v=2:800:600:webp:1704067200" - wantSignature: "394_Vf9TdQFkpQ3XKFDQSyxgqKq8N7mApf2S4QaHqyo=", + // "cdn.example.com:/photos/cat.jpg:token=abc&v=2:800:600:webp:1704067200:85:cover" + wantSignature: "pKgVBOTd_Q_EikI7MNQLC9Q8Hurdxzyv3EIYvVhqc2I=", wantSignedPath: "/v1/image/cdn.example.com/photos/cat.jpg" + "%3Ftoken=abc&v=2/800x600.webp", }, @@ -71,11 +76,34 @@ func goldenVectors() []goldenVector { Width: 0, Height: 0, Format: testFormatPNG, + Quality: 85, + FitMode: testFitCover, }, - // Signed data: "cdn.example.com:/photos/cat.jpg::0:0:png:1704067200" - wantSignature: "7Be7oteeQwvnSPU4bchyQ4ZGYGsAGBKpeEtuQ02ox60=", + // Signed data: + // "cdn.example.com:/photos/cat.jpg::0:0:png:1704067200:85:cover" + wantSignature: "6_rZ0yyVbGZRs8kG7n7HLgLi5Jt8vjiWQljIEL1jbIs=", wantSignedPath: "/v1/image/cdn.example.com/photos/cat.jpg/orig.png", }, + { + name: "non-default quality and fit", + req: signature.Request{ + SourceHost: testHost, + SourcePath: testPath, + SourceQuery: "", + Width: 800, + Height: 600, + Format: testFormatWebP, + Quality: 40, + FitMode: testFitContain, + }, + // Signed data: + // "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200:40:contain" + wantSignature: "pGaXpPUbI3A7nMx-4T9bfq9bYWBNL0kY4bxlcv3g1F8=", + // The path is the same as for the default quality and fit: + // q=40&fit=contain go in the query string next to sig and + // exp (imgcache.Service.GenerateSignedURL adds all four). + wantSignedPath: testSignedPath, + }, } } @@ -84,11 +112,9 @@ func goldenVectors() []goldenVector { // hardcoded signing key. // // If any of these assertions fail, the signed byte format -// ("host:path:query:width:height:format:expiration"), the base64url -// encoding, or the signed URL layout has changed. Such a change breaks -// every signature already issued to clients, so it must be made -// deliberately: update these constants only as part of an intentional, -// documented signature format migration. +// ("host:path:query:width:height:format:expiration:quality:fit"), the +// base64url encoding, or the signed URL layout has changed. Update these +// constants only when that change is intended. func TestSigner_GoldenVectors(t *testing.T) { t.Parallel() 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..0b43032 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. @@ -56,7 +65,8 @@ func New(secretKey string) *Signer { } // Sign generates an HMAC-SHA256 signature for the given request. -// The signature covers: host + path + query + width + height + format + expiration. +// The signature covers: host + path + query + width + height + format + +// expiration + quality + fit. func (s *Signer) Sign(req *Request) string { data := s.buildSignatureData(req) mac := hmac.New(sha256.New, s.secretKey) @@ -68,7 +78,8 @@ func (s *Signer) Sign(req *Request) string { // Verify checks if the signature on the request is valid and not expired. // Signatures are exact-match only: every component of the signed data -// (host, path, query, dimensions, format, expiration) must match exactly. +// (host, path, query, dimensions, format, expiration, quality, fit) must +// match exactly. // No suffix matching, wildcard matching, or partial matching is supported. // A signature for "cdn.example.com" will NOT verify for "example.com" or // "other.cdn.example.com", and vice versa. @@ -142,11 +153,13 @@ func (s *Signer) GenerateSignedURL( } // buildSignatureData creates the string to be signed. -// Format: "host:path:query:width:height:format:expiration" +// Format: "host:path:query:width:height:format:expiration:quality:fit" // All components are used verbatim (exact match). No normalization, -// suffix matching, or wildcard expansion is performed. +// suffix matching, or wildcard expansion is performed. Quality and fit +// are the effective transform values, so replaying a signed URL with a +// different quality or fit mode fails verification. func (s *Signer) buildSignatureData(req *Request) string { - return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d", + return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s", req.SourceHost, req.SourcePath, req.SourceQuery, @@ -154,6 +167,8 @@ func (s *Signer) buildSignatureData(req *Request) string { req.Height, req.Format, req.Expires.Unix(), + req.Quality, + req.FitMode, ) } 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" )