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
clawbot 1798cba96c Take the image signing key from PIXA_SIGNING_KEY and refuse the example placeholder (closes #110)
check / check (push) Failing after 1s
The Docker image now ships config.docker.yml, which sets only signing_key (read from the PIXA_SIGNING_KEY environment variable), state_dir and port. The placeholder key and the five-host allowlist from config.example.yml are no longer in the image; anything else is configured by mounting a file over /etc/pixa/config.yml. A container started without PIXA_SIGNING_KEY exits naming it.

Startup now refuses the exact placeholder signing_key from config.example.yml. It is 45 characters long and used to pass the length check, so a deployment could sign URLs with a key that is public in this repository. README Getting Started is corrected to match.

What a reader would trip over: the unset-variable error comes from config interpolation, not from validate(); the signing key checks moved into validateSigningKey to stay under the complexity limit.

Disclosure: TODO.md is not updated by this change.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 21:59:24 +02:00
clawbot 37d49ade11 Harden http.Server: slowloris timeouts and form body limit (closes #92)
check / check (push) Failing after 1s
The http.Server now sets ReadHeaderTimeout (10s), which bounds the slow header dribble that ReadTimeout alone does not, and IdleTimeout (120s), which bounds keep-alive reuse. Server construction moved into a small helper so a test can assert the timeouts without binding a listener.

POST / and POST /generate bodies are capped at 1 MiB and an oversized body returns 413.

What a reader would trip over: the CSRF library reads its token from the form and swallows a parse error, so a cap applied only inside it would surface as 403. The body limit therefore parses the form under the cap before the CSRF check; the parsed form is reused afterwards. A test covers an oversized body that carries a valid token.

Judgement call: WriteTimeout stays at 60s; it also bounds how long a large image may take to send over a slow link.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 20:59:24 +02:00
clawbot b4e5300feb feat: add HSTS, CSP, and Permissions-Policy security headers (closes #91)
check / check (push) Failing after 0s
SecurityHeaders() now also sets Strict-Transport-Security (one year, includeSubDomains), a Content-Security-Policy (default-src self, frame-ancestors none) and a Permissions-Policy denying the browser features pixa does not use. X-Frame-Options stays as the legacy fallback.

What a reader would trip over: HSTS is sent on every response even though pixa listens on plain HTTP behind a TLS-terminating proxy; browsers ignore the header over plaintext, and this avoids trusting a forwarded-proto header. The clipboard feature is left unlisted so the copy button on the generator page keeps working.

Disclosure: script-src and style-src carry unsafe-inline because the generator template has inline onclick handlers and the bundled Tailwind script injects a style element at runtime; removing it needs template changes and is tracked separately.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 20:43:13 +02:00
16 changed files with 804 additions and 39 deletions
+7 -3
View File
@@ -15,12 +15,16 @@ git clone https://git.eeqj.de/sneak/pixa.git
cd pixa cd pixa
make build make build
# run with a config file # run with a config file: copy the example and set a real signing key
./bin/pixad --config config.example.yml # (the example placeholder is refused at startup), e.g. with
# openssl rand -base64 32
cp config.example.yml config.yml
$EDITOR config.yml # replace the signing_key placeholder
./bin/pixad --config config.yml
# or build and run via Docker # or build and run via Docker
make docker make docker
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixad:latest docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
``` ```
A container is configured two ways. The signing key comes from the A container is configured two ways. The signing key comes from the
+19
View File
@@ -29,6 +29,25 @@ 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
`HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
`HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
existing timeouts and wired them onto the server; added a `LimitBody`
middleware capping the two form POST bodies (`POST /`, `POST /generate`)
at `MaxFormBytes` (1 MiB) and returning 413, applied ahead of the CSRF
middleware so an oversized body is refused as 413 rather than being read
as a missing CSRF token (403); left `WriteTimeout` at 60s unchanged
- 2026-08-07 update golangci-lint to v2.12.2 with the canonical - 2026-08-07 update golangci-lint to v2.12.2 with the canonical
`.golangci.yml` (v2 schema, `default: all` minus six disabled `.golangci.yml` (v2 schema, `default: all` minus six disabled
linters, `lll` 88, tests included): bumped the pinned linters, `lll` 88, tests included): bumped the pinned
+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")
}
}
+45
View File
@@ -0,0 +1,45 @@
package handlers
import (
"errors"
"net/http"
)
// MaxFormBytes bounds the request body accepted on the HTML form POST
// routes (POST / and POST /generate). The forms carry a handful of short
// fields, so 1 MiB is generous while making the bound explicit rather than
// resting on ParseForm's incidental 10 MB cap.
const MaxFormBytes = 1 << 20 // 1 MiB
// LimitBody returns middleware that caps the request body on POST requests
// at maxBytes and rejects an oversized body with 413 Request Entity Too
// Large.
//
// It parses the form here, before the CSRF middleware reads the token from
// it. The CSRF middleware reads the token with PostFormValue, which
// swallows a parse error, so if the body were only capped there an
// oversized body would read as a missing token and be refused as 403. By
// parsing under the cap first, an oversized body is refused as 413. A
// successful parse is cached on the request, so the CSRF check and the
// handler reuse it rather than reading the body again.
func (s *Handlers) LimitBody(maxBytes int64) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPost {
r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
err := r.ParseForm()
var tooLarge *http.MaxBytesError
if errors.As(err, &tooLarge) {
http.Error(w, "Request body too large",
http.StatusRequestEntityTooLarge)
return
}
}
next.ServeHTTP(w, r)
})
}
}
@@ -0,0 +1,177 @@
package handlers
import (
"log/slog"
"net/http"
"net/url"
"strings"
"testing"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/session"
)
// Form field names and a throwaway source image URL for the body-limit
// tests.
const (
sourceURLField = "url"
testSourceURL = "https://example.com/a.jpg"
)
// newBodyLimitTestRouter mirrors the production wiring for the form POST
// routes (see server.SetupRoutes): LimitBody sits in front of the CSRF
// middleware, which sits in front of the handlers. maxBytes is the body
// cap under test, so a test can trip the limit with a small body.
func newBodyLimitTestRouter(
t *testing.T, maxBytes int64,
) (*Handlers, http.Handler) {
t.Helper()
cfg := &config.Config{SigningKey: testSigningKey, Debug: true}
sessMgr, err := session.NewManager(testSigningKey)
if err != nil {
t.Fatalf("session.NewManager() error = %v", err)
}
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
protect, err := newCSRFProtect(testSigningKey, cfg.Debug)
if err != nil {
t.Fatalf("newCSRFProtect() error = %v", err)
}
h := &Handlers{
log: slog.New(slog.DiscardHandler),
config: cfg,
sessMgr: sessMgr,
encGen: encGen,
csrfProtect: protect,
}
r := chi.NewRouter()
r.Group(func(r chi.Router) {
r.Use(h.LimitBody(maxBytes))
r.Use(h.CSRF())
r.Get("/", h.HandleRoot())
r.Post("/", h.HandleRoot())
r.Post("/generate", h.HandleGenerateURL())
})
return h, r
}
// TestOversizedLoginPostRejectedBeforeCSRF is the core regression: an
// oversized POST / carrying an otherwise valid CSRF cookie and token must
// be rejected with 413. If the body limit ran after CSRF, the truncated
// body would read as a missing token and return 403; if it ran after the
// handler, a valid token would return 303. Getting 413 proves the limit
// fires before CSRF parses the form.
func TestOversizedLoginPostRejectedBeforeCSRF(t *testing.T) {
t.Parallel()
_, srv := newBodyLimitTestRouter(t, 16)
cookies, token := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookies, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {token},
})
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("oversized POST / status = %d, want %d",
rec.Code, http.StatusRequestEntityTooLarge)
}
}
// TestOversizedGeneratePostRejectedBeforeCSRF is the same regression for
// POST /generate, which also parses a form behind CSRF.
func TestOversizedGeneratePostRejectedBeforeCSRF(t *testing.T) {
t.Parallel()
h, srv := newBodyLimitTestRouter(t, 16)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
rec := postForm(srv, "/generate", cookies, url.Values{
sourceURLField: {testSourceURL},
csrfTokenField: {token},
})
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("oversized POST /generate status = %d, want %d",
rec.Code, http.StatusRequestEntityTooLarge)
}
}
// TestWithinLimitLoginPostSucceeds verifies the limit does not disturb a
// normal request: under the production cap, a valid login still parses and
// establishes a session (303). This guards against the body limit
// consuming or corrupting the form the CSRF check and handler depend on.
func TestWithinLimitLoginPostSucceeds(t *testing.T) {
t.Parallel()
_, srv := newBodyLimitTestRouter(t, MaxFormBytes)
cookies, token := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookies, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {token},
})
if rec.Code != http.StatusSeeOther {
t.Fatalf("within-limit POST / status = %d, want %d",
rec.Code, http.StatusSeeOther)
}
var authed bool
for _, c := range rec.Result().Cookies() {
if c.Name == session.CookieName && c.Value != "" {
authed = true
}
}
if !authed {
t.Error("within-limit valid login did not set a session cookie")
}
}
// TestWithinLimitGeneratePostSucceeds is the same non-regression check for
// POST /generate.
func TestWithinLimitGeneratePostSucceeds(t *testing.T) {
t.Parallel()
h, srv := newBodyLimitTestRouter(t, MaxFormBytes)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
rec := postForm(srv, "/generate", cookies, url.Values{
sourceURLField: {testSourceURL},
"format": {"jpeg"},
csrfTokenField: {token},
})
if rec.Code != http.StatusOK {
t.Fatalf("within-limit POST /generate status = %d, want %d",
rec.Code, http.StatusOK)
}
if !strings.Contains(rec.Body.String(), "/v1/e/") {
t.Error("within-limit generate response did not contain a generated URL")
}
}
+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)
}
})
}
}
+37
View File
@@ -21,6 +21,33 @@ import (
// CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours). // CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours).
const CORSMaxAgeSeconds = 86400 const CORSMaxAgeSeconds = 86400
// HSTSValue is the Strict-Transport-Security header value: one year with
// includeSubDomains. Emitted unconditionally even though pixa listens plain
// HTTP behind a TLS-terminating proxy; browsers ignore an HSTS header received
// over plaintext (RFC 6797 section 8.1), so it never lies about the connection,
// and emitting it here avoids trusting a forwarded-proto header.
const HSTSValue = "max-age=31536000; includeSubDomains"
// ContentSecurityPolicyValue is the Content-Security-Policy header value.
// default-src 'self' is the baseline and frame-ancestors 'none' is the primary
// clickjacking control. 'unsafe-inline' is required in script-src and style-src
// because the served templates carry inline onclick handlers (generator page)
// and the bundled Tailwind asset injects a runtime <style> element; dropping it
// needs template changes outside this issue's scope.
const ContentSecurityPolicyValue = "default-src 'self'; " +
"script-src 'self' 'unsafe-inline'; " +
"style-src 'self' 'unsafe-inline'; " +
"object-src 'none'; " +
"base-uri 'self'; " +
"form-action 'self'; " +
"frame-ancestors 'none'"
// PermissionsPolicyValue is the Permissions-Policy header value. Every listed
// feature is denied because pixa uses none of them.
const PermissionsPolicyValue = "accelerometer=(), autoplay=(), camera=(), " +
"display-capture=(), geolocation=(), gyroscope=(), magnetometer=(), " +
"microphone=(), payment=(), usb=()"
// Params defines dependencies for Middleware. // Params defines dependencies for Middleware.
type Params struct { type Params struct {
fx.In fx.In
@@ -164,6 +191,16 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
// Disable XSS filtering (modern browsers don't need it, can cause issues) // Disable XSS filtering (modern browsers don't need it, can cause issues)
w.Header().Set("X-XSS-Protection", "0") w.Header().Set("X-XSS-Protection", "0")
// Force HTTPS on future visits (ignored by browsers over plaintext)
w.Header().Set("Strict-Transport-Security", HSTSValue)
// Restrict content sources; frame-ancestors is the primary
// clickjacking control, X-Frame-Options the legacy fallback
w.Header().Set("Content-Security-Policy", ContentSecurityPolicyValue)
// Deny browser features pixa does not use
w.Header().Set("Permissions-Policy", PermissionsPolicyValue)
next.ServeHTTP(w, r) next.ServeHTTP(w, r)
}) })
} }
@@ -56,6 +56,61 @@ func TestSecurityHeaders(t *testing.T) {
} }
} }
func TestSecurityHeaders_PolicyHeaders(t *testing.T) {
t.Parallel()
cfg := &config.Config{}
mw := &Middleware{
log: slog.Default(),
config: cfg,
}
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
handler := mw.SecurityHeaders()(testHandler)
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/test", nil)
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
tests := []struct {
header string
want string
}{
{"Strict-Transport-Security", "max-age=31536000; includeSubDomains"},
{
"Content-Security-Policy",
"default-src 'self'; " +
"script-src 'self' 'unsafe-inline'; " +
"style-src 'self' 'unsafe-inline'; " +
"object-src 'none'; " +
"base-uri 'self'; " +
"form-action 'self'; " +
"frame-ancestors 'none'",
},
{
"Permissions-Policy",
"accelerometer=(), autoplay=(), camera=(), " +
"display-capture=(), geolocation=(), gyroscope=(), " +
"magnetometer=(), microphone=(), payment=(), usb=()",
},
}
for _, tt := range tests {
t.Run(tt.header, func(t *testing.T) {
t.Parallel()
got := rec.Header().Get(tt.header)
if got != tt.want {
t.Errorf("%s = %q, want %q", tt.header, got, tt.want)
}
})
}
}
func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) { func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) {
t.Parallel() t.Parallel()
+27 -11
View File
@@ -9,24 +9,40 @@ import (
// HTTP server configuration constants. // HTTP server configuration constants.
const ( const (
HTTPReadTimeout = 30 * time.Second HTTPReadTimeout = 30 * time.Second
HTTPWriteTimeout = 60 * time.Second // HTTPReadHeaderTimeout bounds the request-header read on its own,
// short, so a slowloris client dribbling headers is dropped well
// before it ties up a connection for the whole ReadTimeout window.
HTTPReadHeaderTimeout = 10 * time.Second
HTTPWriteTimeout = 60 * time.Second
// HTTPIdleTimeout bounds how long an idle keep-alive connection is
// held open, so idle connections cannot accumulate without limit on a
// service targeting high concurrency.
HTTPIdleTimeout = 120 * time.Second
HTTPMaxHeaderBytes = 8 << 10 // 8KB HTTPMaxHeaderBytes = 8 << 10 // 8KB
) )
func (s *Server) serveUntilShutdown() { // newHTTPServer builds the http.Server with the hardening timeouts and
listenAddr := fmt.Sprintf(":%d", s.config.Port) // limits applied. It is separate from serveUntilShutdown so the
s.httpServer = &http.Server{ // configuration can be asserted in a test without binding a listener.
Addr: listenAddr, func (s *Server) newHTTPServer() *http.Server {
ReadTimeout: HTTPReadTimeout, return &http.Server{
WriteTimeout: HTTPWriteTimeout, Addr: fmt.Sprintf(":%d", s.config.Port),
MaxHeaderBytes: HTTPMaxHeaderBytes, ReadTimeout: HTTPReadTimeout,
Handler: s, ReadHeaderTimeout: HTTPReadHeaderTimeout,
WriteTimeout: HTTPWriteTimeout,
IdleTimeout: HTTPIdleTimeout,
MaxHeaderBytes: HTTPMaxHeaderBytes,
Handler: s,
} }
}
func (s *Server) serveUntilShutdown() {
s.httpServer = s.newHTTPServer()
s.SetupRoutes() s.SetupRoutes()
s.log.Info("http begin listen", "listenaddr", listenAddr) s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
err := s.httpServer.ListenAndServe() err := s.httpServer.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) { if err != nil && !errors.Is(err, http.ErrServerClosed) {
+65
View File
@@ -0,0 +1,65 @@
package server
import (
"testing"
"time"
"sneak.berlin/go/pixa/internal/config"
)
// TestNewHTTPServerTimeouts verifies that the constructed http.Server
// carries every hardening timeout wired onto it, including the slowloris
// defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This
// guards against a field being defined but never set on the server, so
// each assertion compares the server field to its constant.
func TestNewHTTPServerTimeouts(t *testing.T) {
t.Parallel()
s := &Server{config: &config.Config{Port: 8080}}
srv := s.newHTTPServer()
fields := []struct {
name string
got time.Duration
want time.Duration
}{
{"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout},
{"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout},
{"WriteTimeout", srv.WriteTimeout, HTTPWriteTimeout},
{"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout},
}
for _, f := range fields {
if f.got != f.want {
t.Errorf("%s = %v, want %v", f.name, f.got, f.want)
}
}
if srv.MaxHeaderBytes != HTTPMaxHeaderBytes {
t.Errorf("MaxHeaderBytes = %d, want %d",
srv.MaxHeaderBytes, HTTPMaxHeaderBytes)
}
if srv.Handler != s {
t.Error("Handler is not the server")
}
}
// TestHardeningTimeoutValues pins the intent behind the two new timeouts
// without hard-coding brittle exact durations: the header-read phase is
// bounded strictly shorter than the whole-request read (the slowloris
// dribble), and idle keep-alive connections are bounded rather than held
// open forever.
func TestHardeningTimeoutValues(t *testing.T) {
t.Parallel()
if HTTPReadHeaderTimeout <= 0 || HTTPReadHeaderTimeout > HTTPReadTimeout {
t.Errorf("ReadHeaderTimeout = %v, want positive and <= ReadTimeout %v",
HTTPReadHeaderTimeout, HTTPReadTimeout)
}
if HTTPIdleTimeout <= 0 {
t.Errorf("IdleTimeout = %v, want positive bound", HTTPIdleTimeout)
}
}
+4 -1
View File
@@ -8,6 +8,7 @@ import (
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp" "github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/pixa/internal/handlers"
"sneak.berlin/go/pixa/internal/static" "sneak.berlin/go/pixa/internal/static"
) )
@@ -46,8 +47,10 @@ func (s *Server) SetupRoutes() {
// Login/generator UI. The form routes carry CSRF protection; the // Login/generator UI. The form routes carry CSRF protection; the
// token cookie is independent of the session cookie, so it also // token cookie is independent of the session cookie, so it also
// covers the login POST, where no session exists yet. // covers the login POST, where no session exists yet. LimitBody caps
// the POST body ahead of CSRF, which reads its token from that body.
s.router.Group(func(r chi.Router) { s.router.Group(func(r chi.Router) {
r.Use(s.h.LimitBody(handlers.MaxFormBytes))
r.Use(s.h.CSRF()) r.Use(s.h.CSRF())
r.Get("/", s.h.HandleRoot()) r.Get("/", s.h.HandleRoot())
r.Post("/", s.h.HandleRoot()) r.Post("/", s.h.HandleRoot())