Fix security vulnerabilities, error handling, and accessibility across codebase

Addresses IDOR in AcceptPartner/RemovePartner, OAuth state CSRF,
XSS in profile messages, cookie Secure flag behind proxy, pervasive
swallowed DB errors, bubble sort perf, slice corruption, missing
indexes, HTMX stored locally, template buffering, rate limiter IP
extraction, goroutine shutdown, and accessibility (aria-labels,
focus-visible, visually-hidden toggle).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: exploded

cmd/wtw/main.go

@@ -38,25 +38,29 @@ type visitor struct {
 }
 
 func newRateLimiter(rate int, window time.Duration) *rateLimiter {
-	rl := &rateLimiter{
+	return &rateLimiter{
 		visitors: make(map[string]*visitor),
 		rate:     rate,
 		window:   window,
 	}
-	go rl.cleanup()
-	return rl
 }
 
-func (rl *rateLimiter) cleanup() {
+func (rl *rateLimiter) cleanup(ctx context.Context) {
+	ticker := time.NewTicker(time.Minute)
+	defer ticker.Stop()
 	for {
-		time.Sleep(time.Minute)
-		rl.mu.Lock()
-		for ip, v := range rl.visitors {
-			if time.Since(v.windowStart) > rl.window {
-				delete(rl.visitors, ip)
+		select {
+		case <-ticker.C:
+			rl.mu.Lock()
+			for ip, v := range rl.visitors {
+				if time.Since(v.windowStart) > rl.window {
+					delete(rl.visitors, ip)
+				}
 			}
+			rl.mu.Unlock()
+		case <-ctx.Done():
+			return
 		}
-		rl.mu.Unlock()
 	}
 }
 
@@ -76,12 +80,27 @@ func (rl *rateLimiter) allow(ip string) bool {
 	return v.count <= rl.rate
 }
 
+// clientIP extracts the client IP, preferring X-Forwarded-For behind a proxy.
+func clientIP(r *http.Request) string {
+	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
+		// First entry is the original client
+		if ip := strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]); ip != "" {
+			return ip
+		}
+	}
+	if xri := r.Header.Get("X-Real-IP"); xri != "" {
+		return strings.TrimSpace(xri)
+	}
+	ip, _, err := net.SplitHostPort(r.RemoteAddr)
+	if err != nil {
+		return r.RemoteAddr
+	}
+	return ip
+}
+
 func rateLimitMiddleware(limiter *rateLimiter, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ip, _, err := net.SplitHostPort(r.RemoteAddr)
-		if err != nil {
-			ip = r.RemoteAddr
-		}
+		ip := clientIP(r)
 		if !limiter.allow(ip) {
 			slog.Warn("rate limit exceeded", "ip", ip)
 			http.Error(w, "Too Many Requests", http.StatusTooManyRequests)
@@ -145,9 +164,13 @@ func main() {
 
 	queries := db.New(database)
 
+	// Cancellable context for background goroutines
+	bgCtx, bgCancel := context.WithCancel(context.Background())
+	defer bgCancel()
+
 	// Session store
 	store := session.NewStore(queries)
-	go store.CleanupLoop()
+	go store.CleanupLoop(bgCtx)
 
 	// Auth handler
 	authHandler := auth.NewHandler(
@@ -156,18 +179,23 @@ func main() {
 		os.Getenv("GOOGLE_REDIRECT_URL"),
 		queries,
 		store,
+		isProd,
 	)
 
-	// TMDB client — refresh poster paths in background
+	// TMDB client -- refresh poster paths in background
 	tmdbClient := tmdb.NewClient(os.Getenv("TMDB_API_KEY"), queries)
-	tmdbClient.StartRefreshLoop(context.Background())
+	tmdbClient.StartRefreshLoop(bgCtx)
 
 	// Recommendation engine
 	engine := recommend.NewEngine(queries, os.Getenv("ANTHROPIC_API_KEY"), tmdbClient)
 
 	// Handlers
 	h := handlers.New(queries, store, engine, tmdbClient)
 
+	// Rate limiter with cancellable cleanup
+	limiter := newRateLimiter(60, time.Minute)
+	go limiter.cleanup(bgCtx)
+
 	// Routes
 	mux := http.NewServeMux()
 
@@ -207,7 +235,6 @@ func main() {
 	})
 
 	// Middleware chain
-	limiter := newRateLimiter(60, time.Minute)
 	handler := middleware.RequestLogger(
 		rateLimitMiddleware(limiter,
 			middleware.SecurityHeaders(isProd, mux),
@@ -222,18 +249,27 @@ func main() {
 		IdleTimeout:  60 * time.Second,
 	}
 
+	// Use a channel to propagate server errors to main goroutine
+	serverErr := make(chan error, 1)
 	go func() {
 		slog.Info("HTTP server listening", "addr", srv.Addr)
 		if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
-			slog.Error("server error", "error", err)
-			os.Exit(1)
+			serverErr <- err
 		}
 	}()
 
 	quit := make(chan os.Signal, 1)
 	signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
-	<-quit
-	slog.Info("shutting down server...")
+
+	select {
+	case <-quit:
+		slog.Info("shutting down server...")
+	case err := <-serverErr:
+		slog.Error("server error", "error", err)
+	}
+
+	// Cancel background goroutines before shutting down the server
+	bgCancel()
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
@@ -246,16 +282,21 @@ func main() {
 // migrateRatingsConstraint recreates the ratings table if the CHECK constraint
 // does not include 'favourite'. Needed because CREATE TABLE IF NOT EXISTS skips
 // existing tables, so the schema update alone cannot fix old databases.
-func migrateRatingsConstraint(db *sql.DB) error {
+func migrateRatingsConstraint(database *sql.DB) error {
 	var tableSql string
-	err := db.QueryRow("SELECT sql FROM sqlite_master WHERE type='table' AND name='ratings'").Scan(&tableSql)
+	err := database.QueryRow("SELECT sql FROM sqlite_master WHERE type='table' AND name='ratings'").Scan(&tableSql)
 	if err != nil {
 		return nil // table doesn't exist yet, schema will create it
 	}
 	if strings.Contains(tableSql, "'favourite'") {
 		return nil // already migrated
 	}
-	_, err = db.Exec(`
+	tx, err := database.Begin()
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+	_, err = tx.Exec(`
 		CREATE TABLE ratings_new (
 			user_id   INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
 			show_id   TEXT    NOT NULL REFERENCES shows(id),
@@ -268,7 +309,10 @@ func migrateRatingsConstraint(db *sql.DB) error {
 		ALTER TABLE ratings_new RENAME TO ratings;
 		CREATE INDEX IF NOT EXISTS idx_ratings_user ON ratings(user_id);
 	`)
-	return err
+	if err != nil {
+		return err
+	}
+	return tx.Commit()
 }
 
 func cacheStaticAssets(next http.Handler) http.Handler {

internal/auth/google.go

@@ -2,7 +2,9 @@ package auth
 
 import (
 	"context"
+	"crypto/rand"
 	"database/sql"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"log/slog"
@@ -19,17 +21,18 @@ type Handler struct {
 	config  *oauth2.Config
 	queries *db.Queries
 	store   *session.Store
+	isProd  bool
 }
 
-func NewHandler(clientID, clientSecret, redirectURL string, queries *db.Queries, store *session.Store) *Handler {
+func NewHandler(clientID, clientSecret, redirectURL string, queries *db.Queries, store *session.Store, isProd bool) *Handler {
 	cfg := &oauth2.Config{
 		ClientID:     clientID,
 		ClientSecret: clientSecret,
 		RedirectURL:  redirectURL,
 		Scopes:       []string{"openid", "email", "profile"},
 		Endpoint:     google.Endpoint,
 	}
-	return &Handler{config: cfg, queries: queries, store: store}
+	return &Handler{config: cfg, queries: queries, store: store, isProd: isProd}
 }
 
 type googleUserInfo struct {
@@ -40,11 +43,42 @@ type googleUserInfo struct {
 }
 
 func (h *Handler) Login(w http.ResponseWriter, r *http.Request) {
-	url := h.config.AuthCodeURL("state-token", oauth2.AccessTypeOffline)
+	b := make([]byte, 16)
+	if _, err := rand.Read(b); err != nil {
+		http.Error(w, "internal error", http.StatusInternalServerError)
+		return
+	}
+	state := hex.EncodeToString(b)
+
+	http.SetCookie(w, &http.Cookie{
+		Name:     "oauth_state",
+		Value:    state,
+		Path:     "/",
+		MaxAge:   600,
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+		Secure:   h.isProd,
+	})
+
+	url := h.config.AuthCodeURL(state, oauth2.AccessTypeOffline)
 	http.Redirect(w, r, url, http.StatusTemporaryRedirect)
 }
 
 func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) {
+	// Validate OAuth state
+	stateCookie, err := r.Cookie("oauth_state")
+	if err != nil || stateCookie.Value == "" || stateCookie.Value != r.URL.Query().Get("state") {
+		http.Error(w, "invalid state", http.StatusBadRequest)
+		return
+	}
+	// Clear the state cookie
+	http.SetCookie(w, &http.Cookie{
+		Name:   "oauth_state",
+		Value:  "",
+		Path:   "/",
+		MaxAge: -1,
+	})
+
 	code := r.URL.Query().Get("code")
 	if code == "" {
 		http.Error(w, "missing code", http.StatusBadRequest)
@@ -99,15 +133,20 @@ func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) {
 		Name:     "session",
 		Value:    sessionToken,
 		Path:     "/",
-		MaxAge:   30 * 24 * 60 * 60,
+		MaxAge:   session.MaxAgeSec,
 		HttpOnly: true,
 		SameSite: http.SameSiteLaxMode,
-		Secure:   r.TLS != nil,
+		Secure:   h.isProd,
 	})
 
 	// Check if user has ratings to decide redirect
 	ratings, err := h.queries.GetUserRatings(r.Context(), user.ID)
-	if err != nil || len(ratings) == 0 {
+	if err != nil {
+		slog.Error("failed to check user ratings", "error", err)
+		http.Redirect(w, r, "/onboarding", http.StatusSeeOther)
+		return
+	}
+	if len(ratings) == 0 {
 		http.Redirect(w, r, "/onboarding", http.StatusSeeOther)
 		return
 	}

internal/db/helpers.go

@@ -10,5 +10,5 @@ func NewNullString(s string) sql.NullString {
 }
 
 func NewNullInt64(i int64) sql.NullInt64 {
-	return sql.NullInt64{Int64: i, Valid: i != 0}
+	return sql.NullInt64{Int64: i, Valid: true}
 }

internal/db/queries.sql

@@ -156,7 +156,7 @@ VALUES (?, ?, ?, ?, ?, ?, ?, ?);
 SELECT * FROM shows WHERE tmdb_id = ?;
 
 -- name: GetShowByTitle :one
-SELECT * FROM shows WHERE LOWER(title) = LOWER(?) LIMIT 1;
+SELECT * FROM shows WHERE title = ? COLLATE NOCASE LIMIT 1;
 
 -- name: CountShows :one
 SELECT COUNT(*) FROM shows;

internal/db/queries.sql.go

@@ -62,6 +62,8 @@ CREATE TABLE IF NOT EXISTS partner_verdicts (
 );
 
 CREATE INDEX IF NOT EXISTS idx_ratings_user ON ratings(user_id);
+CREATE INDEX IF NOT EXISTS idx_ratings_show ON ratings(show_id);
 CREATE INDEX IF NOT EXISTS idx_sessions_expires ON sessions(expires_at);
 CREATE INDEX IF NOT EXISTS idx_partnerships_user ON partnerships(user_id);
 CREATE INDEX IF NOT EXISTS idx_partnerships_email ON partnerships(partner_email);
+CREATE INDEX IF NOT EXISTS idx_partnerships_partner ON partnerships(partner_id);

internal/db/schema.sql

@@ -1,6 +1,7 @@
 package db
 
 import (
+	"context"
 	"database/sql"
 	"fmt"
 )
@@ -68,11 +69,17 @@ var SeedShows = []SeedShow{
 }
 
 func SeedAllShows(database *sql.DB) error {
+	queries := New(database)
+	ctx := context.Background()
 	for _, s := range SeedShows {
-		_, err := database.Exec(
-			`INSERT OR IGNORE INTO shows (id, title, year, poster_path, genre, popularity) VALUES (?, ?, ?, ?, ?, ?)`,
-			s.ID, s.Title, s.Year, s.PosterPath, s.Genre, s.Popularity,
-		)
+		err := queries.InsertShow(ctx, InsertShowParams{
+			ID:         s.ID,
+			Title:      s.Title,
+			Year:       s.Year,
+			PosterPath: NewNullString(s.PosterPath),
+			Genre:      NewNullString(s.Genre),
+			Popularity: sql.NullFloat64{Float64: s.Popularity, Valid: true},
+		})
 		if err != nil {
 			return fmt.Errorf("seeding show %s: %w", s.ID, err)
 		}