2 Commits
Author SHA1 Message Date
sneak fa1018b614 docs: update TODO.md Workflow and Status for the next branching model (closes #106)
check / check (push) Successful in 3m23s
The Workflow section still told contributors to branch from `main` and
merge there. Rewrite it to the current model: branch per issue from
`next`, PR based on `next`, an independent reviewer gates it, the
manager squash-merges into `next`, and only the owner merges `next`
into `main` via the milestone PR. Update the Status paragraph so it no
longer says work is green on `main`, and record that the `1.0.0`
milestone is in progress with `next` at the canonical golangci-lint
v2.12.2 config. Docs-only; the rest of the file is left unreflowed
(that is issue 100).

Model: opus-4-8
2026-09-21 17:29:03 +00:00
clawbot a96eba8083 CSRF protection on the login and URL-generator forms (closes #93)
check / check (push) Failing after 1s
Adds CSRF protection to the two cookie-authenticated form posts, POST / (login) and POST /generate, using github.com/gorilla/csrf, the recorded default for this job.

The token key is derived from signing_key with its own HKDF salt, so it needs no new config and survives restarts. The token cookie is separate from the session cookie, which also covers login CSRF, where no session exists yet. Both templates carry the hidden token field.

What a reader would trip over: outside debug mode the library enforces its https Referer origin check, so the TLS-terminating proxy must preserve the Host and Referer headers from the browser or form posts are rejected.

Disclosure: one nolint:gosec on a test constant holding the library field name (G101 false positive).

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 19:26:18 +02:00
10 changed files with 410 additions and 38 deletions
+13 -7
View File
@@ -1,21 +1,27 @@
# Workflow
* branch (from `main`)
* branch per issue from `next`
* do the work in Next Step
* move Next Step to the top of Completed Steps
* move the top item of Future Steps into Next Step
* commit (`TODO.md` changes in the same commit as the work)
* merge to `main` if the branch is not protected, otherwise open a PR
* open a PR based on `next`
* an independent reviewer who did not write the change gates it
* the manager squash-merges the PR into `next` once review passes
* `next` stays green and mergeable to `main` at any time; only the owner
merges `next` into `main`, via the single milestone PR
* push
# Status
pre-1.0. No git tags exist. Recent work extracted the internal/magic,
pre-1.0. No git tags exist. The `1.0.0` milestone is in progress; work
lands on `next`, and `main` receives only the milestone PR that the
owner merges. `next` is at the canonical `golangci-lint` v2.12.2 config
and is green. Recent work extracted the internal/magic,
internal/allowlist, internal/httpfetcher, and internal/signature
packages. The gosec findings from the 2026-07-06 survey are resolved
and `make check` is green on main. The disk cache is now size-bounded
with LRU eviction (`cache_max_bytes`), closing the unbounded disk
growth DoS vector.
packages. The gosec findings from the 2026-07-06 survey are resolved.
The disk cache is now size-bounded with LRU eviction
(`cache_max_bytes`), closing the unbounded disk growth DoS vector.
# Next Step
+1
View File
@@ -11,6 +11,7 @@ require (
github.com/getsentry/sentry-go v0.40.0
github.com/go-chi/chi/v5 v5.2.3
github.com/go-chi/cors v1.2.2
github.com/gorilla/csrf v1.7.3
github.com/gorilla/securecookie v1.1.2
github.com/prometheus/client_golang v1.23.2
github.com/slok/go-http-metrics v0.13.0
+2
View File
@@ -175,6 +175,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU
github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA=
github.com/googleapis/gax-go/v2 v2.14.2 h1:eBLnkZ9635krYIPD+ag1USrOAI0Nr0QYF3+/3GqO0k0=
github.com/googleapis/gax-go/v2 v2.14.2/go.mod h1:ON64QhlJkhVtSqp4v1uaK92VyZ2gmvDQsweuyLV+8+w=
github.com/gorilla/csrf v1.7.3 h1:BHWt6FTLZAb2HtWT5KDBf6qgpZzvtbp9QWDRKZMXJC0=
github.com/gorilla/csrf v1.7.3/go.mod h1:F1Fj3KG23WYHE6gozCmBAezKookxbIvUJT+121wTuLk=
github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=
github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 h1:5ZPtiqj0JL5oKWmcsq4VMaAW5ukBEgSGXEN89zeH1Jo=
+25 -15
View File
@@ -2,6 +2,7 @@ package handlers
import (
"crypto/subtle"
"html/template"
"net/http"
"net/url"
"strconv"
@@ -23,13 +24,13 @@ func (s *Handlers) HandleRoot() http.HandlerFunc {
// Check if authenticated
if s.sessMgr.IsAuthenticated(r) {
s.renderGenerator(w, nil)
s.renderGenerator(w, r, nil)
return
}
// Show login page
s.renderLogin(w, "")
s.renderLogin(w, r, "")
}
}
@@ -37,7 +38,7 @@ func (s *Handlers) HandleRoot() http.HandlerFunc {
func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
err := r.ParseForm()
if err != nil {
s.renderLogin(w, "Invalid form data")
s.renderLogin(w, r, "Invalid form data")
return
}
@@ -47,7 +48,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
// Constant-time comparison to prevent timing attacks
if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 {
s.log.Warn("failed login attempt", "remote_addr", r.RemoteAddr)
s.renderLogin(w, "Invalid signing key")
s.renderLogin(w, r, "Invalid signing key")
return
}
@@ -56,7 +57,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
err = s.sessMgr.CreateSession(w)
if err != nil {
s.log.Error("failed to create session", "error", err)
s.renderLogin(w, "Failed to create session")
s.renderLogin(w, r, "Failed to create session")
return
}
@@ -87,7 +88,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
err := r.ParseForm()
if err != nil {
s.renderGenerator(w, &generatorData{Error: "Invalid form data"})
s.renderGenerator(w, r, &generatorData{Error: "Invalid form data"})
return
}
@@ -97,7 +98,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
// Validate source URL
parsed, err := url.Parse(sourceURL)
if err != nil || parsed.Host == "" {
s.renderGeneratorWithForm(w, "Invalid source URL", r.Form)
s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form)
return
}
@@ -108,7 +109,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
token, err := s.encGen.Generate(payload)
if err != nil {
s.log.Error("failed to generate encrypted URL", "error", err)
s.renderGeneratorWithForm(w, "Failed to generate URL", r.Form)
s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form)
return
}
@@ -121,7 +122,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
expiresAtStr = expiresAt.Format(time.RFC3339)
}
s.renderGenerator(w, &generatorData{
s.renderGenerator(w, r, &generatorData{
GeneratedURL: generatedURL,
ExpiresAt: expiresAtStr,
FormURL: sourceURL,
@@ -186,15 +187,20 @@ type generatorData struct {
FormQuality string
FormFit string
FormTTL string
CSRFField template.HTML
}
func (s *Handlers) renderLogin(w http.ResponseWriter, errorMsg string) {
func (s *Handlers) renderLogin(
w http.ResponseWriter, r *http.Request, errorMsg string,
) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
data := struct {
Error string
Error string
CSRFField template.HTML
}{
Error: errorMsg,
Error: errorMsg,
CSRFField: csrfField(r),
}
err := templates.Render(w, "login.html", data)
@@ -204,13 +210,17 @@ func (s *Handlers) renderLogin(w http.ResponseWriter, errorMsg string) {
}
}
func (s *Handlers) renderGenerator(w http.ResponseWriter, data *generatorData) {
func (s *Handlers) renderGenerator(
w http.ResponseWriter, r *http.Request, data *generatorData,
) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if data == nil {
data = &generatorData{}
}
data.CSRFField = csrfField(r)
err := templates.Render(w, "generator.html", data)
if err != nil {
s.log.Error("failed to render generator template", "error", err)
@@ -219,9 +229,9 @@ func (s *Handlers) renderGenerator(w http.ResponseWriter, data *generatorData) {
}
func (s *Handlers) renderGeneratorWithForm(
w http.ResponseWriter, errorMsg string, form url.Values,
w http.ResponseWriter, r *http.Request, errorMsg string, form url.Values,
) {
s.renderGenerator(w, &generatorData{
s.renderGenerator(w, r, &generatorData{
Error: errorMsg,
FormURL: form.Get("url"),
FormWidth: form.Get("width"),
@@ -0,0 +1,273 @@
package handlers
import (
"context"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"regexp"
"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"
)
// testSigningKey is a throwaway signing key for the CSRF flow tests. It
// seeds the session manager, the encrypted-URL generator, and the CSRF
// token key, exactly as the real signing key does in production.
const testSigningKey = "test-signing-key-0123456789abcdef"
// Form field names used in the CSRF flow tests.
const (
loginKeyField = "key"
// gorilla/csrf's default form field name, not a credential.
csrfTokenField = "gorilla.csrf.Token" //nolint:gosec // G101 false positive
)
// csrfFieldPattern extracts the token rendered by csrf.TemplateField into
// the form. The field name is gorilla/csrf's default.
var csrfFieldPattern = regexp.MustCompile(
`name="gorilla\.csrf\.Token" value="([^"]+)"`)
// newCSRFTestRouter builds a router that mirrors the production wiring for
// the CSRF-protected UI routes (see server.SetupRoutes): the login and
// generator forms and their POST targets sit behind the real CSRF
// middleware. Requests are marked plaintext (Debug: true) so the flow runs
// over httptest's http transport without an https Referer.
func newCSRFTestRouter(t *testing.T) (*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.CSRF())
r.Get("/", h.HandleRoot())
r.Post("/", h.HandleRoot())
r.Post("/generate", h.HandleGenerateURL())
})
return h, r
}
// csrfCredentials performs a GET that renders a form and returns the CSRF
// cookies the middleware set and the token embedded in the form. Passing
// the authenticated session cookie renders the generator form instead of
// the login form.
func csrfCredentials(
t *testing.T, srv http.Handler, reqCookies []*http.Cookie,
) ([]*http.Cookie, string) {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, "/", nil)
for _, c := range reqCookies {
req.AddCookie(c)
}
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("GET / status = %d, want %d", rec.Code, http.StatusOK)
}
match := csrfFieldPattern.FindStringSubmatch(rec.Body.String())
if match == nil {
t.Fatalf("no CSRF token field found in rendered form")
}
return rec.Result().Cookies(), match[1]
}
// postForm submits form values with the given cookies and returns the
// recorder.
func postForm(
srv http.Handler, path string,
cookies []*http.Cookie, form url.Values,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, path,
strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
for _, c := range cookies {
req.AddCookie(c)
}
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, req)
return rec
}
// TestLoginPostRejectedWithoutToken verifies that POST / with no CSRF token
// is rejected. This is login CSRF: no session cookie exists yet, so the
// protection must rest on a token bound to a pre-session cookie.
func TestLoginPostRejectedWithoutToken(t *testing.T) {
t.Parallel()
_, srv := newCSRFTestRouter(t)
rec := postForm(srv, "/", nil, url.Values{loginKeyField: {testSigningKey}})
if rec.Code != http.StatusForbidden {
t.Errorf("POST / without token status = %d, want %d",
rec.Code, http.StatusForbidden)
}
}
// TestLoginPostRejectedWithForeignToken verifies that a token that does not
// match the request's CSRF cookie is rejected: a token minted for one
// cookie cannot authorize a request carrying a different cookie.
func TestLoginPostRejectedWithForeignToken(t *testing.T) {
t.Parallel()
_, srv := newCSRFTestRouter(t)
cookiesA, _ := csrfCredentials(t, srv, nil)
_, tokenB := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookiesA, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {tokenB},
})
if rec.Code != http.StatusForbidden {
t.Errorf("POST / with foreign token status = %d, want %d",
rec.Code, http.StatusForbidden)
}
}
// TestLoginPostAcceptedWithValidToken verifies that POST / with a matching
// cookie and token succeeds: the login is processed and a session is
// established (303 redirect).
func TestLoginPostAcceptedWithValidToken(t *testing.T) {
t.Parallel()
_, srv := newCSRFTestRouter(t)
cookies, token := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookies, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {token},
})
if rec.Code != http.StatusSeeOther {
t.Fatalf("POST / with valid token 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("valid login did not set a session cookie")
}
}
// TestGeneratePostRejectedWithoutToken verifies that POST /generate is
// rejected without a CSRF token even when the request carries a valid
// authenticated session. The session cookie is not sufficient; the policy
// requires a CSRF token on this cookie-authenticated form.
func TestGeneratePostRejectedWithoutToken(t *testing.T) {
t.Parallel()
h, srv := newCSRFTestRouter(t)
sessionCookie := newSessionCookie(t, h)
rec := postForm(srv, "/generate",
[]*http.Cookie{sessionCookie},
url.Values{"url": {"https://example.com/a.jpg"}})
if rec.Code != http.StatusForbidden {
t.Errorf("POST /generate without token status = %d, want %d",
rec.Code, http.StatusForbidden)
}
}
// TestGeneratePostAcceptedWithValidToken verifies that POST /generate
// succeeds with a valid session and a matching CSRF cookie and token.
func TestGeneratePostAcceptedWithValidToken(t *testing.T) {
t.Parallel()
h, srv := newCSRFTestRouter(t)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
rec := postForm(srv, "/generate", cookies, url.Values{
"url": {"https://example.com/a.jpg"},
"format": {"jpeg"},
csrfTokenField: {token},
})
if rec.Code != http.StatusOK {
t.Fatalf("POST /generate with valid token status = %d, want %d",
rec.Code, http.StatusOK)
}
if !strings.Contains(rec.Body.String(), "/v1/e/") {
t.Error("generator response did not contain a generated URL")
}
}
// newSessionCookie creates an authenticated session cookie via the
// handler's session manager.
func newSessionCookie(t *testing.T, h *Handlers) *http.Cookie {
t.Helper()
rec := httptest.NewRecorder()
err := h.sessMgr.CreateSession(rec)
if err != nil {
t.Fatalf("CreateSession() error = %v", err)
}
for _, c := range rec.Result().Cookies() {
if c.Name == session.CookieName {
return c
}
}
t.Fatalf("session manager did not set a %q cookie", session.CookieName)
return nil
}
+65
View File
@@ -0,0 +1,65 @@
package handlers
import (
"html/template"
"net/http"
"github.com/gorilla/csrf"
"sneak.berlin/go/pixa/internal/seal"
)
// csrfKeySalt provides domain separation for the CSRF authentication key,
// derived from the signing key so tokens survive restarts without extra
// configuration and never reuse the session or encrypted-URL key material.
const csrfKeySalt = "pixa-csrf-v1"
// newCSRFProtect builds the CSRF-protection middleware for the
// state-mutating HTML form routes. The token lives in its own cookie,
// independent of the session cookie, so it also protects the login POST
// where no session exists yet (login CSRF).
//
// When plaintext is true (local HTTP development), requests are marked
// plaintext so the library neither demands an https Referer nor sets a
// Secure cookie the browser would withhold over http. In production the
// service runs behind a TLS-terminating proxy, so plaintext is false and
// the library enforces its https Referer origin check.
func newCSRFProtect(
signingKey string, plaintext bool,
) (func(http.Handler) http.Handler, error) {
key, err := seal.DeriveKey([]byte(signingKey), csrfKeySalt)
if err != nil {
return nil, err
}
protect := csrf.Protect(
key[:],
csrf.Path("/"),
csrf.Secure(!plaintext),
csrf.SameSite(csrf.SameSiteStrictMode),
)
if !plaintext {
return protect, nil
}
return func(next http.Handler) http.Handler {
protected := protect(next)
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
protected.ServeHTTP(w, csrf.PlaintextHTTPRequest(r))
})
}, nil
}
// CSRF returns the CSRF-protection middleware for the login and generator
// form routes.
func (s *Handlers) CSRF() func(http.Handler) http.Handler {
return s.csrfProtect
}
// csrfField returns the hidden form input carrying the CSRF token for the
// given request, to be embedded verbatim in a rendered form.
func csrfField(r *http.Request) template.HTML {
return csrf.TemplateField(r)
}
+19 -12
View File
@@ -31,23 +31,30 @@ type Params struct {
// Handlers provides HTTP request handlers.
type Handlers struct {
log *slog.Logger
hc *healthcheck.Healthcheck
db *database.Database
config *config.Config
imgSvc *imgcache.Service
imgCache *imgcache.Cache
sessMgr *session.Manager
encGen *encurl.Generator
log *slog.Logger
hc *healthcheck.Healthcheck
db *database.Database
config *config.Config
imgSvc *imgcache.Service
imgCache *imgcache.Cache
sessMgr *session.Manager
encGen *encurl.Generator
csrfProtect func(http.Handler) http.Handler
}
// New creates a new Handlers instance.
func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
csrfProtect, err := newCSRFProtect(params.Config.SigningKey, params.Config.Debug)
if err != nil {
return nil, err
}
s := &Handlers{
log: params.Logger.Get(),
hc: params.Healthcheck,
db: params.Database,
config: params.Config,
log: params.Logger.Get(),
hc: params.Healthcheck,
db: params.Database,
config: params.Config,
csrfProtect: csrfProtect,
}
lc.Append(fx.Hook{
+10 -4
View File
@@ -44,11 +44,17 @@ func (s *Server) SetupRoutes() {
// Static files (Tailwind CSS, etc.)
s.router.Handle("/static/*", http.StripPrefix("/static/", static.Handler()))
// Login/generator UI
s.router.Get("/", s.h.HandleRoot())
s.router.Post("/", s.h.HandleRoot())
// Login/generator UI. The form routes carry CSRF protection; the
// token cookie is independent of the session cookie, so it also
// covers the login POST, where no session exists yet.
s.router.Group(func(r chi.Router) {
r.Use(s.h.CSRF())
r.Get("/", s.h.HandleRoot())
r.Post("/", s.h.HandleRoot())
r.Post("/generate", s.h.HandleGenerateURL())
})
s.router.Get("/logout", s.h.HandleLogout())
s.router.Post("/generate", s.h.HandleGenerateURL())
// Main image proxy route
// /v1/image/<host>/<path>/<width>x<height>.<format>
+1
View File
@@ -47,6 +47,7 @@
{{end}}
<form method="POST" action="/generate" class="bg-white rounded-lg shadow-md p-6 space-y-4">
{{ .CSRFField }}
<div>
<label for="url" class="block text-sm font-medium text-gray-700 mb-1">
Source URL
+1
View File
@@ -17,6 +17,7 @@
{{end}}
<form method="POST" action="/" class="space-y-4">
{{ .CSRFField }}
<div>
<label for="key" class="block text-sm font-medium text-gray-700 mb-1">
Signing Key