Harden sessions, uploads, admin demotion, and HTTP timeouts.

Address PR review findings: renew session tokens on auth, sniff/re-encode avatars, serialize last-admin checks, bound server timeouts, rune-safe truncation, and TEST_DATABASE_URL-only integration tests.

.env.example

@@ -3,6 +3,8 @@ LISTEN=:8080
 # Required: PlanetScale Postgres URI (port 5432 so the app can create tables on boot).
 # Switch to 6432 (PgBouncer) later if you need pooling.
 DATABASE_URL=postgresql://user:password@host.example.com:5432/postgres?sslmode=verify-full
+# Required for integration tests (do not point at the runtime DATABASE_URL).
+# TEST_DATABASE_URL=postgresql://user:password@host.example.com:5432/plumber_test?sslmode=verify-full
 # Optional: first matching registrant becomes admin only if no admin exists yet.
 # Later promote/demote via /admin/users (admins only).
 ADMIN_USERNAME=yourusername

cmd/server/main.go

@@ -29,7 +29,14 @@ func main() {
 
 	uploader := blob.FromEnv()
 	handler := newHandler(db, sessions, uploader)
-	run(&http.Server{Addr: listenAddr(), Handler: handler})
+	run(&http.Server{
+		Addr:              listenAddr(),
+		Handler:           handler,
+		ReadHeaderTimeout: 5 * time.Second,
+		ReadTimeout:       30 * time.Second,
+		WriteTimeout:      60 * time.Second,
+		IdleTimeout:       90 * time.Second,
+	})
 }
 
 func openDB() (*sql.DB, *store.SessionStore) {

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/jackc/pgx/v5 v5.10.0
 	github.com/joho/godotenv v1.5.1
 	golang.org/x/crypto v0.55.0
+	golang.org/x/image v0.45.0
 )
 
 require (

go.sum

@@ -50,6 +50,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M=
 golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis=
+golang.org/x/image v0.45.0 h1:FMb1nTbH5H9vF55SriQHgFw5GnNL9Jg6L25BwXKzhB0=
+golang.org/x/image v0.45.0/go.mod h1:n62x/7RqlwXDvGsSU4u6IUTUf6KghUZ9Bt7cG/T9Fx4=
 golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
 golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
 golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8=

internal/store/sessions_test.go

@@ -9,10 +9,7 @@ import (
 func TestSessionStoreCommitFindDelete(t *testing.T) {
 	url := os.Getenv("TEST_DATABASE_URL")
 	if url == "" {
-		url = os.Getenv("DATABASE_URL")
-	}
-	if url == "" {
-		t.Skip("DATABASE_URL or TEST_DATABASE_URL not set")
+		t.Skip("TEST_DATABASE_URL not set")
 	}
 	schema, err := os.ReadFile("../../schema.sql")
 	if err != nil {

internal/store/user.go

@@ -92,6 +92,10 @@ func (u *User) Create(ctx context.Context) error {
 	})
 }
 
+// adminRoleLockKey serializes SetRole so concurrent demotions cannot bypass the
+// last-admin guard under READ COMMITTED.
+const adminRoleLockKey int64 = 0x706c756d5f61646d // "plum_adm"
+
 // SetRole updates this user's role (last-admin safe).
 func (u *User) SetRole(ctx context.Context, role Role) error {
 	if u == nil || u.db == nil {
@@ -106,6 +110,10 @@ func (u *User) SetRole(ctx context.Context, role Role) error {
 	}
 	defer tx.Rollback()
 
+	if _, err := tx.ExecContext(ctx, `SELECT pg_advisory_xact_lock($1)`, adminRoleLockKey); err != nil {
+		return err
+	}
+
 	q := sqlc.New(tx)
 	current, err := q.GetUserRole(ctx, u.ID)
 	if err != nil {

internal/web/auth.go

@@ -54,6 +54,10 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 		})
 		return
 	}
+	if err := s.sessions.RenewToken(r.Context()); err != nil {
+		http.Error(w, "could not start session", http.StatusInternalServerError)
+		return
+	}
 	s.sessions.Put(r.Context(), "user_id", u.ID)
 	http.Redirect(w, r, next, http.StatusSeeOther)
 }
@@ -108,6 +112,10 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
 		s.exec(w, "register", p)
 		return
 	}
+	if err := s.sessions.RenewToken(r.Context()); err != nil {
+		http.Error(w, "could not start session", http.StatusInternalServerError)
+		return
+	}
 	s.sessions.Put(r.Context(), "user_id", u.ID)
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }

internal/web/profile.go

@@ -1,12 +1,18 @@
 package web
 
 import (
+	"bytes"
+	"fmt"
+	"image"
+	"image/jpeg"
+	"image/png"
 	"io"
 	"net/http"
 	"path"
 	"strings"
 
 	"github.com/google/uuid"
+	_ "golang.org/x/image/webp"
 
 	"plumber/internal/blob"
 	"plumber/internal/geo"
@@ -63,23 +69,21 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) {
 			s.renderProfile(w, r, u, "Avatar uploads are not configured on this server.", state)
 			return
 		}
-		ct := hdr.Header.Get("Content-Type")
-		ext, contentType, ok := avatarType(hdr.Filename, ct)
-		if !ok {
-			s.renderProfile(w, r, u, "Avatar must be a JPEG, PNG, or WebP image.", state)
-			return
-		}
 		if hdr.Size > 2<<20 {
 			s.renderProfile(w, r, u, "Avatar must be 2MB or smaller.", state)
 			return
 		}
+		body, ext, contentType, prepErr := prepareAvatar(file, 2<<20)
+		if prepErr != nil {
+			s.renderProfile(w, r, u, "Avatar must be a JPEG, PNG, or WebP image.", state)
+			return
+		}
 		key := path.Join("avatars", u.ID, uuid.NewString()+ext)
-		limited := io.LimitReader(file, (2<<20)+1)
 		url, upErr := s.cfg.Blob.Upload(r.Context(), blob.FileUpload{
 			Key:         key,
-			Body:        limited,
+			Body:        bytes.NewReader(body),
 			ContentType: contentType,
-			Size:        hdr.Size,
+			Size:        int64(len(body)),
 		})
 		if upErr != nil {
 			s.renderProfile(w, r, u, "Could not upload avatar. Try again later.", state)
@@ -103,18 +107,49 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/profile", http.StatusSeeOther)
 }
 
-func avatarType(filename, contentType string) (ext, normalized string, ok bool) {
-	contentType = strings.ToLower(strings.TrimSpace(contentType))
-	filename = strings.ToLower(filename)
+// prepareAvatar reads at most maxBytes, sniffs/decodes the image, and re-encodes
+// it so only valid image bytes are stored publicly.
+func prepareAvatar(r io.Reader, maxBytes int64) (body []byte, ext, contentType string, err error) {
+	limited := io.LimitReader(r, maxBytes+1)
+	raw, err := io.ReadAll(limited)
+	if err != nil {
+		return nil, "", "", err
+	}
+	if int64(len(raw)) > maxBytes {
+		return nil, "", "", fmt.Errorf("avatar too large")
+	}
+	if len(raw) == 0 {
+		return nil, "", "", fmt.Errorf("empty avatar")
+	}
+
+	sniff := http.DetectContentType(raw)
 	switch {
-	case strings.HasPrefix(contentType, "image/jpeg"), strings.HasSuffix(filename, ".jpg"), strings.HasSuffix(filename, ".jpeg"):
-		return ".jpg", "image/jpeg", true
-	case strings.HasPrefix(contentType, "image/png"), strings.HasSuffix(filename, ".png"):
-		return ".png", "image/png", true
-	case strings.HasPrefix(contentType, "image/webp"), strings.HasSuffix(filename, ".webp"):
-		return ".webp", "image/webp", true
+	case strings.HasPrefix(sniff, "image/jpeg"),
+		strings.HasPrefix(sniff, "image/png"),
+		strings.HasPrefix(sniff, "image/webp"):
 	default:
-		return "", "", false
+		return nil, "", "", fmt.Errorf("unsupported type %s", sniff)
+	}
+
+	img, format, err := image.Decode(bytes.NewReader(raw))
+	if err != nil {
+		return nil, "", "", err
+	}
+
+	var out bytes.Buffer
+	switch format {
+	case "jpeg":
+		if err := jpeg.Encode(&out, img, &jpeg.Options{Quality: 90}); err != nil {
+			return nil, "", "", err
+		}
+		return out.Bytes(), ".jpg", "image/jpeg", nil
+	case "png", "webp":
+		if err := png.Encode(&out, img); err != nil {
+			return nil, "", "", err
+		}
+		return out.Bytes(), ".png", "image/png", nil
+	default:
+		return nil, "", "", fmt.Errorf("unsupported format %s", format)
 	}
 }
 

internal/web/server.go

@@ -311,13 +311,13 @@ func (s *Server) handleSubmit(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if len(title) > 120 {
-		title = title[:120]
+		title = truncateRunes(title, 120)
 	}
 	if len(body) > 8000 {
-		body = body[:8000]
+		body = truncateRunes(body, 8000)
 	}
 	if len(city) > 80 {
-		city = city[:80]
+		city = truncateRunes(city, 80)
 	}
 	q := store.NewQuestion(s.db)
 	q.AuthorID = u.ID
@@ -449,7 +449,7 @@ func (s *Server) handleAnswer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if len(body) > 12000 {
-		body = body[:12000]
+		body = truncateRunes(body, 12000)
 	}
 	ans := store.NewAnswer(s.db)
 	ans.QuestionID = id
@@ -518,6 +518,21 @@ func (s *Server) exec(w http.ResponseWriter, name string, data any) {
 	}
 }
 
+// truncateRunes shortens s to at most max runes without splitting a code point.
+func truncateRunes(s string, max int) string {
+	if max <= 0 {
+		return ""
+	}
+	n := 0
+	for byteIdx := range s {
+		if n == max {
+			return s[:byteIdx]
+		}
+		n++
+	}
+	return s
+}
+
 func randomHex(n int) string {
 	b := make([]byte, n)
 	if _, err := rand.Read(b); err != nil {

internal/web/server_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
+	"image"
+	"image/png"
 	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
@@ -23,17 +25,14 @@ import (
 
 func testDBURL() string {
 	_ = godotenv.Load()
-	if u := strings.TrimSpace(os.Getenv("TEST_DATABASE_URL")); u != "" {
-		return u
-	}
-	return strings.TrimSpace(os.Getenv("DATABASE_URL"))
+	return strings.TrimSpace(os.Getenv("TEST_DATABASE_URL"))
 }
 
 func newTestServer(t *testing.T, cfg Config) (*Server, *sql.DB) {
 	t.Helper()
 	url := testDBURL()
 	if url == "" {
-		t.Skip("set TEST_DATABASE_URL or DATABASE_URL for web tests")
+		t.Skip("set TEST_DATABASE_URL for web tests")
 	}
 	db, sessions, err := store.OpenPostgres(url, plumber.SchemaSQL)
 	if err != nil {
@@ -162,7 +161,7 @@ func TestRegisterLoginAsk(t *testing.T) {
 func TestSessionSurvivesServerRestart(t *testing.T) {
 	url := testDBURL()
 	if url == "" {
-		t.Skip("set TEST_DATABASE_URL or DATABASE_URL for web tests")
+		t.Skip("set TEST_DATABASE_URL for web tests")
 	}
 	db, sessions, err := store.OpenPostgres(url, plumber.SchemaSQL)
 	if err != nil {
@@ -511,7 +510,10 @@ func TestProfileAdminAnsweredListAndAvatarUpload(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	_, _ = part.Write([]byte("fakepngbytes"))
+	img := image.NewRGBA(image.Rect(0, 0, 1, 1))
+	if err := png.Encode(part, img); err != nil {
+		t.Fatal(err)
+	}
 	_ = w.Close()
 	req = httptest.NewRequest(http.MethodPost, "/profile", &buf)
 	req.Header.Set("Content-Type", w.FormDataContentType())

todo.md

@@ -6,7 +6,7 @@ From the project review. Priority order within each section.
 
 - [x] **Persist sessions** — Sessions live in the app DB (`sessions` table) via `postgresstore`. Opaque cookie unchanged; unused `SESSION_SECRET` removed from config / `.env.example`.
 - [x] **Drop Dockerfile** — Deploying on DigitalOcean App Platform (buildpack from `go.mod`); no container image needed.
-- [ ] **Rune-safe truncation** — `title[:120]`, `body[:8000]`, `city[:80]`, answer body, etc. can split multi-byte UTF-8. Truncate by runes (or safely).
+- [x] **Rune-safe truncation** — `title[:120]`, `body[:8000]`, `city[:80]`, answer body, etc. can split multi-byte UTF-8. Truncate by runes (or safely).
 - [x] **Admin bootstrap** — `ADMIN_USERNAME` seeds the first admin on register only when no admin exists. Promote/demote via `/admin/users` (admins only); roles stay in `users.role`.
 
 ## Docs & ops