Author SHA1 Message Date
sneak a663669286 fix: validate dimensions and fit mode on encrypted URLs (closes #62)
check / check (push) Successful in 2m30s
The encrypted /v1/e/ route built its image request straight from the
decrypted payload, so a token could request an over-limit dimension
(reaching libvips and exhausting memory) or an unknown fit mode
(surfacing as a 500 from the processor). The token generator discarded
every strconv.Atoi error, silently turning non-numeric width, height,
quality, or ttl into 0 and applying no upper bound on dimensions.

Add a shared ValidateImageRequest in internal/imgcache enforcing the
MaxDimension bound and ValidateFitMode, and apply it on both the plain
image route and the encrypted route so both reject an over-limit size or
an unrecognized fit mode with 400. The generator now parses each numeric
field explicitly and returns 400 naming the field for non-numeric or
out-of-range input, with width and height bounds-checked so an unusable
token cannot be minted.

Model: opus-4-8
2026-09-21 20:17:08 +00:00
sneak 6d380fbf98 test: cover encrypted-URL and generator validation gaps (#62)
Failing tests for the missing validation on the encrypted-URL path and
the token generator: an encrypted token carrying an over-limit dimension
or an unrecognized fit mode must be rejected with 400, and POST /generate
with a non-numeric or over-limit width must return 400 rather than
minting a token. Also covers the shared imgcache validator directly.

Model: opus-4-8
2026-09-21 20:17:08 +00:00
8 changed files with 379 additions and 24 deletions
+11
View File
@@ -29,6 +29,17 @@ P1: implement blocked networks configuration to extend SSRF protection
# Completed Steps # Completed Steps
- 2026-09-21 validate dimensions and fit mode on the encrypted-URL
route and the token generator (closes #62): added a shared
`ValidateImageRequest` in `internal/imgcache` enforcing the
`MaxDimension` bound and `ValidateFitMode`, applied by both the
`/v1/image/` and `/v1/e/` routes, so an over-limit size or an unknown
fit mode is a 400 rather than an out-of-memory or a 500 from the
processor; the URL generator now checks every numeric form field and
rejects a non-numeric or out-of-range `width`, `height`, `quality`,
or `ttl` with a 400 naming the field instead of coercing it to `0`,
and `width`/`height` are bounds-checked so an unusable token cannot be
minted
- 2026-09-21 http.Server hardening (closes #92): added - 2026-09-21 http.Server hardening (closes #92): added
`HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and `HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
`HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the `HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
+98 -17
View File
@@ -2,6 +2,8 @@ package handlers
import ( import (
"crypto/subtle" "crypto/subtle"
"errors"
"fmt"
"html/template" "html/template"
"net/http" "net/http"
"net/url" "net/url"
@@ -13,6 +15,11 @@ import (
"sneak.berlin/go/pixa/internal/templates" "sneak.berlin/go/pixa/internal/templates"
) )
// errInvalidFormField reports a generator form field 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")
// HandleRoot serves the login page or generator page based on authentication state. // HandleRoot serves the login page or generator page based on authentication state.
func (s *Handlers) HandleRoot() http.HandlerFunc { func (s *Handlers) HandleRoot() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
@@ -98,18 +105,26 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
// Validate source URL // Validate source URL
parsed, err := url.Parse(sourceURL) parsed, err := url.Parse(sourceURL)
if err != nil || parsed.Host == "" { if err != nil || parsed.Host == "" {
s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form) s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form,
http.StatusBadRequest)
return return
} }
payload, expiresAt, ttl := buildGeneratePayload(parsed, r.Form) payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form)
if err != nil {
s.renderGeneratorWithForm(w, r, err.Error(), r.Form,
http.StatusBadRequest)
return
}
// Generate encrypted token // Generate encrypted token
token, err := s.encGen.Generate(payload) token, err := s.encGen.Generate(payload)
if err != nil { if err != nil {
s.log.Error("failed to generate encrypted URL", "error", err) s.log.Error("failed to generate encrypted URL", "error", err)
s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form) s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form,
http.StatusInternalServerError)
return return
} }
@@ -137,17 +152,38 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
} }
// buildGeneratePayload parses the numeric form fields and assembles the // buildGeneratePayload parses the numeric form fields and assembles the
// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). // encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A
// non-numeric or out-of-range field, or an unrecognized fit mode, is a client
// error naming the offending field, so an unusable token is never minted.
func buildGeneratePayload( func buildGeneratePayload(
parsed *url.URL, form url.Values, parsed *url.URL, form url.Values,
) (*encurl.Payload, time.Time, int) { ) (*encurl.Payload, time.Time, int, error) {
width, _ := strconv.Atoi(form.Get("width")) width, err := parseFormDimension(form, "width")
height, _ := strconv.Atoi(form.Get("height")) if err != nil {
quality, _ := strconv.Atoi(form.Get("quality")) return nil, time.Time{}, 0, err
ttl, _ := strconv.Atoi(form.Get("ttl")) }
if quality <= 0 { height, err := parseFormDimension(form, "height")
quality = 85 if err != nil {
return nil, time.Time{}, 0, err
}
quality, err := parseFormCount(form, "quality", encurl.DefaultQuality)
if err != nil {
return nil, time.Time{}, 0, err
}
ttl, err := parseFormCount(form, "ttl", 0)
if err != nil {
return nil, time.Time{}, 0, err
}
fitMode := imgcache.FitMode(form.Get("fit"))
err = imgcache.ValidateFitMode(fitMode)
if err != nil {
return nil, time.Time{}, 0,
fmt.Errorf("%w: %s", imgcache.ErrInvalidFitMode, form.Get("fit"))
} }
var ( var (
@@ -168,11 +204,45 @@ func buildGeneratePayload(
Height: height, Height: height,
Format: imgcache.ImageFormat(form.Get("format")), Format: imgcache.ImageFormat(form.Get("format")),
Quality: quality, Quality: quality,
FitMode: imgcache.FitMode(form.Get("fit")), FitMode: fitMode,
ExpiresAt: expiresAtUnix, ExpiresAt: expiresAtUnix,
} }
return payload, expiresAt, ttl return payload, expiresAt, ttl, nil
}
// parseFormDimension reads an optional width or height form field. An empty
// value means "original size" (0). A non-numeric, negative, or over-limit
// value is rejected with an error naming the field.
func parseFormDimension(form url.Values, field string) (int, error) {
raw := form.Get(field)
if raw == "" {
return 0, nil
}
value, err := strconv.Atoi(raw)
if err != nil || value < 0 || value > imgcache.MaxDimension {
return 0, fmt.Errorf("%w %s", errInvalidFormField, field)
}
return value, nil
}
// parseFormCount reads an optional non-negative integer form field, returning
// def when the field is empty and an error naming the field when the value is
// non-numeric or negative.
func parseFormCount(form url.Values, field string, def int) (int, error) {
raw := form.Get(field)
if raw == "" {
return def, nil
}
value, err := strconv.Atoi(raw)
if err != nil || value < 0 {
return 0, fmt.Errorf("%w %s", errInvalidFormField, field)
}
return value, nil
} }
// generatorData holds template data for the generator page. // generatorData holds template data for the generator page.
@@ -212,6 +282,15 @@ func (s *Handlers) renderLogin(
func (s *Handlers) renderGenerator( func (s *Handlers) renderGenerator(
w http.ResponseWriter, r *http.Request, data *generatorData, w http.ResponseWriter, r *http.Request, data *generatorData,
) {
s.renderGeneratorStatus(w, r, data, http.StatusOK)
}
// renderGeneratorStatus renders the generator page with an explicit HTTP
// status. The status is written before the body so both it and the
// Content-Type header take effect; a rejected form uses 400.
func (s *Handlers) renderGeneratorStatus(
w http.ResponseWriter, r *http.Request, data *generatorData, status int,
) { ) {
w.Header().Set("Content-Type", "text/html; charset=utf-8") w.Header().Set("Content-Type", "text/html; charset=utf-8")
@@ -221,17 +300,19 @@ func (s *Handlers) renderGenerator(
data.CSRFField = csrfField(r) data.CSRFField = csrfField(r)
w.WriteHeader(status)
err := templates.Render(w, "generator.html", data) err := templates.Render(w, "generator.html", data)
if err != nil { if err != nil {
s.log.Error("failed to render generator template", "error", err) s.log.Error("failed to render generator template", "error", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
} }
} }
func (s *Handlers) renderGeneratorWithForm( func (s *Handlers) renderGeneratorWithForm(
w http.ResponseWriter, r *http.Request, errorMsg string, form url.Values, w http.ResponseWriter, r *http.Request, errorMsg string,
form url.Values, status int,
) { ) {
s.renderGenerator(w, r, &generatorData{ s.renderGeneratorStatus(w, r, &generatorData{
Error: errorMsg, Error: errorMsg,
FormURL: form.Get("url"), FormURL: form.Get("url"),
FormWidth: form.Get("width"), FormWidth: form.Get("width"),
@@ -240,7 +321,7 @@ func (s *Handlers) renderGeneratorWithForm(
FormQuality: form.Get("quality"), FormQuality: form.Get("quality"),
FormFit: form.Get("fit"), FormFit: form.Get("fit"),
FormTTL: form.Get("ttl"), FormTTL: form.Get("ttl"),
}) }, status)
} }
func (s *Handlers) buildGeneratedURL(r *http.Request, token, format string) string { func (s *Handlers) buildGeneratedURL(r *http.Request, token, format string) string {
@@ -0,0 +1,66 @@
package handlers
import (
"maps"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
)
// generatePost submits the /generate form with a valid session and CSRF token
// plus the caller's extra fields, returning the recorder.
func generatePost(
t *testing.T, extra url.Values,
) *httptest.ResponseRecorder {
t.Helper()
h, srv := newCSRFTestRouter(t)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
form := url.Values{
sourceURLField: {testSourceURL},
csrfTokenField: {token},
}
maps.Copy(form, extra)
return postForm(srv, "/generate", cookies, form)
}
// TestGeneratePostRejectsNonNumericWidth verifies that a non-numeric width is
// rejected with 400 naming the field rather than being coerced to 0 and
// minting a 0-width token.
func TestGeneratePostRejectsNonNumericWidth(t *testing.T) {
t.Parallel()
rec := generatePost(t, url.Values{"width": {"abc"}})
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(rec.Body.String(), "/v1/e/") {
t.Error("a token was generated for non-numeric width")
}
}
// TestGeneratePostRejectsOverLimitWidth verifies that a width beyond
// MaxDimension is rejected at generation time so an unusable token cannot be
// minted.
func TestGeneratePostRejectsOverLimitWidth(t *testing.T) {
t.Parallel()
rec := generatePost(t, url.Values{"width": {"100000"}})
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(rec.Body.String(), "/v1/e/") {
t.Error("a token was generated for an over-limit width")
}
}
+12 -7
View File
@@ -110,13 +110,6 @@ func (s *Handlers) parseImageRequest(
if fit := query.Get("fit"); fit != "" { if fit := query.Get("fit"); fit != "" {
req.FitMode = imgcache.FitMode(fit) req.FitMode = imgcache.FitMode(fit)
fitErr := imgcache.ValidateFitMode(req.FitMode)
if fitErr != nil {
s.respondError(w, "invalid fit mode: "+fit, http.StatusBadRequest)
return nil, false
}
} }
// Default quality if not set // Default quality if not set
@@ -129,6 +122,18 @@ func (s *Handlers) parseImageRequest(
req.FitMode = imgcache.FitCover req.FitMode = imgcache.FitCover
} }
// Enforce dimension and fit-mode bounds, shared with the encrypted-URL
// route. Dimensions are already bounded by the path parser above; this
// also rejects an unrecognized fit mode with 400 instead of letting it
// reach the processor as a 500.
err = imgcache.ValidateImageRequest(req)
if err != nil {
s.respondError(w, "invalid image request: "+err.Error(),
http.StatusBadRequest)
return nil, false
}
return req, true return req, true
} }
+13
View File
@@ -50,6 +50,19 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
// Convert payload to ImageRequest // Convert payload to ImageRequest
req := payload.ToImageRequest() req := payload.ToImageRequest()
// Apply the same dimension and fit-mode bounds as the plain image
// route: a sealed payload is trusted for its origin, not for staying
// within limits, so an over-limit size or unknown fit mode is a 400
// here rather than an out-of-memory or a 500 from the processor.
err = imgcache.ValidateImageRequest(req)
if err != nil {
s.log.Debug("encrypted URL failed validation", "error", err)
s.respondError(w, "invalid encrypted URL: "+err.Error(),
http.StatusBadRequest)
return
}
// Log the request // Log the request
s.log.Debug("encrypted image request", s.log.Debug("encrypted image request",
"host", req.SourceHost, "host", req.SourceHost,
@@ -0,0 +1,98 @@
package handlers
import (
"context"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// newEncTestServer builds a router serving the encrypted-URL route with a
// generator seeded by the shared test signing key. The image service is left
// nil: these tests exercise validation that rejects a token before any image
// is fetched, so the handler must never reach the service.
func newEncTestServer(t *testing.T) (*encurl.Generator, http.Handler) {
t.Helper()
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
h := &Handlers{
log: slog.New(slog.DiscardHandler),
encGen: encGen,
}
r := chi.NewRouter()
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
return encGen, r
}
// getEncToken issues a GET for the given token and returns the recorder.
func getEncToken(srv http.Handler, token string) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, "/v1/e/"+token+"/img.jpg", nil)
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, req)
return rec
}
// TestHandleImageEnc_OverLimitDimension_Returns400 verifies that a decrypted
// token requesting a dimension beyond MaxDimension is rejected with 400
// instead of reaching the image processor and libvips.
func TestHandleImageEnc_OverLimitDimension_Returns400(t *testing.T) {
t.Parallel()
encGen, srv := newEncTestServer(t)
token, err := encGen.Generate(&encurl.Payload{
SourceHost: "cdn.example.com",
SourcePath: "/photo.jpg",
Width: 100000,
Height: 100000,
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
rec := getEncToken(srv, token)
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
}
// TestHandleImageEnc_InvalidFitMode_Returns400 verifies that a decrypted token
// carrying an unrecognized fit mode is rejected with 400 rather than surfacing
// as a 500 from the image processor's default branch.
func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) {
t.Parallel()
encGen, srv := newEncTestServer(t)
token, err := encGen.Generate(&encurl.Payload{
SourceHost: "cdn.example.com",
SourcePath: "/photo.jpg",
Width: 800,
Height: 600,
FitMode: imgcache.FitMode("bogus"),
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
rec := getEncToken(srv, token)
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
}
+17
View File
@@ -59,6 +59,23 @@ func ValidateFitMode(fit FitMode) error {
} }
} }
// ValidateImageRequest checks that a request's dimensions are within
// MaxDimension and its fit mode is recognized. Both the plain /v1/image/
// route and the encrypted /v1/e/ route validate through this function so a
// request from either source enforces identical bounds, regardless of how it
// was constructed. A width or height of 0 means "original size" and is valid.
func ValidateImageRequest(req *ImageRequest) error {
if req.Size.Width < 0 || req.Size.Height < 0 {
return ErrInvalidSize
}
if req.Size.Width > MaxDimension || req.Size.Height > MaxDimension {
return ErrDimensionTooLarge
}
return ValidateFitMode(req.FitMode)
}
// ImageRequest represents a request for a processed image // ImageRequest represents a request for a processed image
type ImageRequest struct { type ImageRequest struct {
// SourceHost is the origin host (e.g., "cdn.example.com") // SourceHost is the origin host (e.g., "cdn.example.com")
@@ -0,0 +1,64 @@
package imgcache
import (
"errors"
"testing"
)
func TestValidateImageRequest(t *testing.T) {
t.Parallel()
tests := []struct {
name string
req ImageRequest
wantErr error
}{
{
name: "within bounds",
req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: FitCover},
},
{
name: "original size and empty fit",
req: ImageRequest{Size: Size{Width: 0, Height: 0}},
},
{
name: "width over limit",
req: ImageRequest{Size: Size{Width: MaxDimension + 1, Height: 600}},
wantErr: ErrDimensionTooLarge,
},
{
name: "height over limit",
req: ImageRequest{Size: Size{Width: 800, Height: MaxDimension + 1}},
wantErr: ErrDimensionTooLarge,
},
{
name: "negative width",
req: ImageRequest{Size: Size{Width: -1, Height: 600}},
wantErr: ErrInvalidSize,
},
{
name: "invalid fit mode",
req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: "bogus"},
wantErr: ErrInvalidFitMode,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := ValidateImageRequest(&tt.req)
if tt.wantErr == nil {
if err != nil {
t.Fatalf("ValidateImageRequest() error = %v, want nil", err)
}
return
}
if !errors.Is(err, tt.wantErr) {
t.Fatalf("ValidateImageRequest() error = %v, want %v", err, tt.wantErr)
}
})
}
}