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
11 changed files with 395 additions and 125 deletions
+2 -5
View File
@@ -13,12 +13,9 @@ RUN go mod download
# Copy source code
COPY . .
# Run formatting check and linter. The linter is invoked directly, not
# via `make lint`: `make lint` now builds Dockerfile.lint, and there is
# no Docker inside a Docker build. This is the same linter, image, and
# config that Dockerfile.lint and script/lint run.
# Run formatting check and linter
RUN make fmt-check
RUN golangci-lint run --config .golangci.yml ./...
RUN make lint
# Build stage
# golang:1.25.4-alpine, 2026-02-25
-41
View File
@@ -1,41 +0,0 @@
# Dockerfile.lint: the one and only path that runs golangci-lint.
#
# golangci-lint is never installed on the host; it runs only inside this
# build. A clean build of this file therefore IS a clean lint over the
# whole tree. It runs the same linter and config as Dockerfile's lint
# stage, pinned to the same image so the two cannot drift to different
# linter versions.
#
# golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60
# pixa is CGO/libvips: the type-aware linters compile every package, so
# this image needs the same C libraries the build does.
RUN apk add --no-cache build-base vips-dev libheif-dev pkgconfig
WORKDIR /src
# Modules first for layer caching; go.mod/go.sum settle this layer's
# result, so it may safely be reused between runs.
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Caching is deliberately waived for the lint step: an unchanged tree
# must still run the linter, not return a cached success in well under a
# second having linted nothing. CACHEBUST carries a value that differs
# on every run (script/lint supplies it and refuses to build without
# one). The lint RUN below references it, so BuildKit cannot serve that
# step from cache. Keep the ${CACHEBUST} reference on that step: dropping
# it lets the linter cache again and report a green that linted nothing.
ARG CACHEBUST
RUN test -n "${CACHEBUST}" || { \
echo "Dockerfile.lint requires the CACHEBUST build-arg; build it via script/lint." >&2; \
exit 1; }
# `golangci-lint config verify` is deliberately not run: it fetches its
# JSON schema over an unpinned live HTTPS call, which REPO_POLICIES.md
# forbids for external references.
RUN echo "pixa-lint: running golangci-lint (${CACHEBUST})" && \
golangci-lint run --config .golangci.yml ./...
+11 -9
View File
@@ -29,15 +29,17 @@ P1: implement blocked networks configuration to extend SSRF protection
# Completed Steps
- 2026-09-21 run all linting in Docker via `Dockerfile.lint` +
`script/lint` (closes #104): `script/lint` builds a hash-pinned root
`Dockerfile.lint`, and no host or nix-shell `golangci-lint` path
remains; a per-run `CACHEBUST` build-arg forces the lint step to
execute every run, so an unchanged tree cannot return a cached green
that linted nothing; `Dockerfile`'s lint stage runs `golangci-lint`
directly, since `make lint` now builds a container and there is no
Docker inside a build; `golangci-lint config verify` stays out, as it
fetches its schema over an unpinned live HTTPS call
- 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
`HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
`HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
+98 -17
View File
@@ -2,6 +2,8 @@ package handlers
import (
"crypto/subtle"
"errors"
"fmt"
"html/template"
"net/http"
"net/url"
@@ -13,6 +15,11 @@ import (
"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.
func (s *Handlers) HandleRoot() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@@ -98,18 +105,26 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
// Validate source URL
parsed, err := url.Parse(sourceURL)
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
}
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
token, err := s.encGen.Generate(payload)
if err != nil {
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
}
@@ -137,17 +152,38 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
}
// 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(
parsed *url.URL, form url.Values,
) (*encurl.Payload, time.Time, int) {
width, _ := strconv.Atoi(form.Get("width"))
height, _ := strconv.Atoi(form.Get("height"))
quality, _ := strconv.Atoi(form.Get("quality"))
ttl, _ := strconv.Atoi(form.Get("ttl"))
) (*encurl.Payload, time.Time, int, error) {
width, err := parseFormDimension(form, "width")
if err != nil {
return nil, time.Time{}, 0, err
}
if quality <= 0 {
quality = 85
height, err := parseFormDimension(form, "height")
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 (
@@ -168,11 +204,45 @@ func buildGeneratePayload(
Height: height,
Format: imgcache.ImageFormat(form.Get("format")),
Quality: quality,
FitMode: imgcache.FitMode(form.Get("fit")),
FitMode: fitMode,
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.
@@ -212,6 +282,15 @@ func (s *Handlers) renderLogin(
func (s *Handlers) renderGenerator(
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")
@@ -221,17 +300,19 @@ func (s *Handlers) renderGenerator(
data.CSRFField = csrfField(r)
w.WriteHeader(status)
err := templates.Render(w, "generator.html", data)
if err != nil {
s.log.Error("failed to render generator template", "error", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
}
}
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,
FormURL: form.Get("url"),
FormWidth: form.Get("width"),
@@ -240,7 +321,7 @@ func (s *Handlers) renderGeneratorWithForm(
FormQuality: form.Get("quality"),
FormFit: form.Get("fit"),
FormTTL: form.Get("ttl"),
})
}, status)
}
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 != "" {
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
@@ -129,6 +122,18 @@ func (s *Handlers) parseImageRequest(
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
}
+13
View File
@@ -50,6 +50,19 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
// Convert payload to ImageRequest
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
s.log.Debug("encrypted image request",
"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
type ImageRequest struct {
// 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)
}
})
}
}
+14 -46
View File
@@ -1,55 +1,23 @@
#!/bin/sh
# script/lint: run golangci-lint over the whole tree.
#
# The linter is never installed on the host: it runs only inside the
# Dockerfile.lint build, one way, everywhere. A clean build is a clean
# lint. See Dockerfile.lint for why the lint step cannot be cached.
# script/lint: run the linter. CGO dependencies (pkg-config, vips,
# libheif) come from nix-shell when not already available (e.g. inside
# a Docker build or an existing nix-shell).
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
# A value no other run repeats. Dockerfile.lint folds it into the
# lint step's cache key, so the linter re-executes every run instead
# of an unchanged tree returning a cached success having linted
# nothing.
cachebust="$(date +%s)-$$"
tmp="$(mktemp -d "${TMPDIR:-/tmp}/pixa-lint.XXXXXX")"
trap 'rm -rf "$tmp"' EXIT INT TERM
# --progress=plain so the lint step's own output reaches the log we
# check below; --output=type=cacheonly because we want the linter's
# verdict, not an image left in the local store. The build status
# travels through a file: a pipeline's exit status is tee's, not the
# build's.
(
set +e
docker build \
--progress=plain \
--build-arg CACHEBUST="$cachebust" \
--output=type=cacheonly \
-f Dockerfile.lint . 2>&1
echo "$?" >"$tmp/status"
) | tee "$tmp/build.log"
status="$(cat "$tmp/status" 2>/dev/null || echo 1)"
[ "${status:-1}" -eq 0 ] || exit "${status:-1}"
# The linter's start line must appear as build output, not only in
# the build's echo of the RUN instruction. A step served from cache
# prints the instruction and none of its output; a step that runs
# prints a "#<n> <elapsed> ..." output line. Requiring that output
# line means a future edit dropping the CACHEBUST reference from
# Dockerfile.lint fails here rather than passing having linted
# nothing.
if ! grep -Eq '^#[0-9]+ +[0-9]+\.[0-9]+ +pixa-lint: running golangci-lint' \
"$tmp/build.log"; then
echo "script/lint: golangci-lint did not execute (cached step?)." >&2
exit 1
run_with_cgo_deps() {
if command -v pkg-config >/dev/null 2>&1; then
sh -c "$1"
else
nix-shell -p pkg-config vips libheif golangci-lint git --run "$1"
fi
}
main() {
cd "$ROOT"
echo "Running linter..."
run_with_cgo_deps "golangci-lint run"
}
main "$@"