Harden security: per-user sort, proxy-aware rate limit, drop dev bypass

Addresses findings from a self-review of the codebase:

- clientIP only trusts X-Forwarded-For when RemoteAddr is loopback, so the
  rate limiter buckets per real client behind Caddy without becoming
  spoofable on a direct-to-Go deployment.
- Remove DEV_USER_EMAIL OAuth bypass entirely. Local dev now uses real
  Google OAuth like prod.
- exercises.sort_order is no longer global. New user_exercise_sort_order
  table holds per-user overrides; ListExercisesForUser falls back to the
  seeded default. /settings/reorder writes per-user rows scoped to the
  caller, so one user can no longer reorder everyone else's home page.
- Cap in-flight contact-email goroutines with a 2-slot semaphore so a
  flood can't fan out enough concurrent SMTP sends to exhaust fds or trip
  Gmail's per-account rate limit.
- Add defence-in-depth user_id WHERE clauses to UpdateSetActualReps,
  UpdateSetsWeightForExercise, and UpsertWalkingSession (handlers already
  verify ownership; this catches future regressions at the SQL layer).
- Log DeleteSession errors on logout instead of swallowing them.
- Pin appleboy/scp-action and appleboy/ssh-action to commit SHAs so a
  retagged release can't grab the deploy SSH key.

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

Author: James McHugh

.env.example

@@ -11,10 +11,6 @@ OAUTH_REDIRECT_URL=http://localhost:8080/auth/google/callback
 # Random 32 bytes hex - generate with: openssl rand -hex 32
 SESSION_KEY=replace_with_64_random_hex_chars
 
-# Optional: shortcut to bypass Google OAuth in local dev. If set,
-# /auth/login auto-creates this user and signs them in. NEVER set in prod.
-DEV_USER_EMAIL=
-
 # Gmail SMTP for the /contact form. Generate an app password at
 # https://myaccount.google.com/apppasswords (2-Step Verification must be on).
 # Leave SMTP_USER/SMTP_PASS blank in local dev to log submissions to stdout

.github/workflows/deploy.yml

@@ -48,7 +48,7 @@ jobs:
         run: CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags="-s -w" -o train .
 
       - name: Copy files to server
-        uses: appleboy/scp-action@v0.1.7
+        uses: appleboy/scp-action@917f8b81dfc1ccd331fef9e2d61bdc6c8be94634 # v0.1.7
         with:
           host: ${{ secrets.DEPLOY_HOST }}
           username: ${{ secrets.DEPLOY_USER }}
@@ -60,7 +60,7 @@ jobs:
 
       - name: Stop service before deploy
         continue-on-error: true
-        uses: appleboy/ssh-action@v1.0.3
+        uses: appleboy/ssh-action@029f5b4aeeeb58fdfe1410a5d17f967dacf36262 # v1.0.3
         with:
           host: ${{ secrets.DEPLOY_HOST }}
           username: ${{ secrets.DEPLOY_USER }}
@@ -69,7 +69,7 @@ jobs:
           script: sudo systemctl stop train
 
       - name: Install and restart service
-        uses: appleboy/ssh-action@v1.0.3
+        uses: appleboy/ssh-action@029f5b4aeeeb58fdfe1410a5d17f967dacf36262 # v1.0.3
         with:
           host: ${{ secrets.DEPLOY_HOST }}
           username: ${{ secrets.DEPLOY_USER }}

README.md

@@ -15,7 +15,9 @@ iPhone-only mobile barbell workout tracker with auto-progression. Hosted at
 
 1. `cp .env.example .env`
 2. Set `SESSION_KEY` to 64 random hex chars (`openssl rand -hex 32`).
-3. Set `DEV_USER_EMAIL=james67@gmail.com` to bypass Google OAuth.
+3. Create a Google OAuth client ID at console.cloud.google.com (redirect URI
+   `http://localhost:8080/auth/google/callback`) and fill in
+   `GOOGLE_CLIENT_ID` / `GOOGLE_CLIENT_SECRET` / `OAUTH_REDIRECT_URL`.
 4. `build.bat` (Windows) — builds and runs on http://localhost:8080.
 5. Use Chrome DevTools iPhone emulation (Cmd+Opt+I → toggle device).
 

auth.go

@@ -33,15 +33,13 @@ type oauthConfig struct {
 	cfg        *oauth2.Config
 	verifier   *oidc.IDTokenVerifier
 	sessionKey []byte
-	devEmail   string // if set, /auth/login bypasses Google entirely
 }
 
 func initOAuth(ctx context.Context) error {
 	clientID := os.Getenv("GOOGLE_CLIENT_ID")
 	clientSecret := os.Getenv("GOOGLE_CLIENT_SECRET")
 	redirect := os.Getenv("OAUTH_REDIRECT_URL")
 	keyHex := os.Getenv("SESSION_KEY")
-	devEmail := os.Getenv("DEV_USER_EMAIL")
 
 	if keyHex == "" {
 		return errors.New("SESSION_KEY not set")
@@ -51,15 +49,10 @@ func initOAuth(ctx context.Context) error {
 		return errors.New("SESSION_KEY must be hex with at least 16 bytes (32 hex chars)")
 	}
 	oauthCfg.sessionKey = key
-	oauthCfg.devEmail = devEmail
 
 	if clientID == "" || clientSecret == "" || redirect == "" {
-		// Allow startup so the dev shortcut still works without OAuth creds.
 		oauthCfg.enabled = false
-		if devEmail == "" {
-			return errors.New("GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, OAUTH_REDIRECT_URL must be set (or DEV_USER_EMAIL for local dev)")
-		}
-		return nil
+		return errors.New("GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, OAUTH_REDIRECT_URL must be set")
 	}
 
 	provider, err := oidc.NewProvider(ctx, "https://accounts.google.com")
@@ -120,16 +113,6 @@ func handleTermsPage(w http.ResponseWriter, r *http.Request) {
 }
 
 func handleAuthLogin(w http.ResponseWriter, r *http.Request) {
-	// Local-dev shortcut: skip Google entirely, log in as DEV_USER_EMAIL.
-	if oauthCfg.devEmail != "" {
-		if err := loginDevUser(r.Context(), w); err != nil {
-			serverError(w, "dev login", err)
-			return
-		}
-		http.Redirect(w, r, "/", http.StatusSeeOther)
-		return
-	}
-
 	if !oauthCfg.enabled {
 		http.Error(w, "OAuth not configured", http.StatusServiceUnavailable)
 		return
@@ -245,7 +228,11 @@ func cleanupExpiredSessions() {
 
 func handleAuthLogout(w http.ResponseWriter, r *http.Request) {
 	if c, err := r.Cookie(sessionCookieName); err == nil {
-		_ = queries.DeleteSession(r.Context(), c.Value)
+		// Log delete failures so a DB outage doesn't silently leave a live
+		// session row behind after the cookie has been cleared client-side.
+		if err := queries.DeleteSession(r.Context(), c.Value); err != nil {
+			slog.Error("logout: delete session", "error", err)
+		}
 	}
 	http.SetCookie(w, &http.Cookie{
 		Name:     sessionCookieName,
@@ -259,16 +246,6 @@ func handleAuthLogout(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/login", http.StatusSeeOther)
 }
 
-func loginDevUser(ctx context.Context, w http.ResponseWriter) error {
-	email := oauthCfg.devEmail
-	sub := "dev:" + email
-	user, err := upsertUser(ctx, sub, email, "Dev User")
-	if err != nil {
-		return err
-	}
-	return issueSessionCookie(ctx, w, user.ID, false)
-}
-
 func upsertUser(ctx context.Context, sub, email, name string) (db.User, error) {
 	now := time.Now().UTC().Format(time.RFC3339)
 	user, err := queries.GetUserByGoogleSub(ctx, sub)

charts.go

@@ -278,7 +278,7 @@ func formatOneDecimal(v float64) string { return strconv.FormatFloat(v, 'f', 1,
 
 func handleCharts(w http.ResponseWriter, r *http.Request) {
 	user := userFrom(r)
-	exercises, err := queries.ListExercises(r.Context())
+	exercises, err := queries.ListExercisesForUser(r.Context(), user.ID)
 	if err != nil {
 		serverError(w, "charts: list exercises", err)
 		return

contact.go

@@ -13,6 +13,13 @@ import (
 
 var contactLimiter = newRateLimiter(3, time.Hour)
 
+// contactSendSem caps in-flight SMTP sends. SMTP send does TLS + auth and
+// can stall for tens of seconds; without a cap a flood of accepted
+// submissions could fan out to enough concurrent goroutines to exhaust file
+// descriptors or trip Gmail's per-account rate limit. The buffer is small
+// because the contact form is low-volume by design.
+var contactSendSem = make(chan struct{}, 2)
+
 type contactPageData struct {
 	Sent  bool
 	Error string
@@ -61,11 +68,20 @@ func handleContactSubmit(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	go func() {
-		if err := sendContactEmail(replyEmail, message, ip); err != nil {
-			slog.Error("contact send", "error", err, "ip", ip)
-		}
-	}()
+	select {
+	case contactSendSem <- struct{}{}:
+		go func() {
+			defer func() { <-contactSendSem }()
+			if err := sendContactEmail(replyEmail, message, ip); err != nil {
+				slog.Error("contact send", "error", err, "ip", ip)
+			}
+		}()
+	default:
+		// Send queue is full. The user still sees a success page (so we
+		// don't reveal capacity to attackers) but the message is dropped
+		// with a warning logged.
+		slog.Warn("contact send dropped: queue full", "ip", ip)
+	}
 
 	redirectContact(w, r, "sent=1")
 }
@@ -103,9 +119,10 @@ func sendContactEmail(replyEmail, message, ip string) error {
 		displayReply = replyEmail
 	}
 
+	submitted := time.Now().In(appLocation).Format("Mon 2 Jan 2006, 3:04 PM MST")
 	body := fmt.Sprintf(
 		"Reply email: %s\nIP: %s\nSubmitted: %s\n\n%s\n",
-		displayReply, ip, time.Now().UTC().Format(time.RFC3339), message,
+		displayReply, ip, submitted, message,
 	)
 
 	msg := []byte(strings.Join([]string{

db/models.go

@@ -49,9 +49,19 @@ DELETE FROM sessions WHERE expires_at <= ?;
 -- =============================================================================
 
 -- name: ListExercises :many
+-- Global order. Used by progression code where order does not matter.
 SELECT id, slug, name, kind, default_sets, default_reps, default_weight_kg, sort_order
 FROM exercises ORDER BY sort_order;
 
+-- name: ListExercisesForUser :many
+-- Per-user order: rows in user_exercise_sort_order override exercises.sort_order.
+-- Exercises without a per-user row fall back to the seeded default.
+SELECT e.id, e.slug, e.name, e.kind, e.default_sets, e.default_reps, e.default_weight_kg, e.sort_order
+FROM exercises e
+LEFT JOIN user_exercise_sort_order uso
+    ON uso.exercise_id = e.id AND uso.user_id = ?
+ORDER BY COALESCE(uso.sort_order, e.sort_order), e.id;
+
 -- name: GetExerciseByID :one
 SELECT id, slug, name, kind, default_sets, default_reps, default_weight_kg, sort_order
 FROM exercises WHERE id = ?;
@@ -67,8 +77,14 @@ ON CONFLICT(slug) DO UPDATE SET
     default_sets = excluded.default_sets,
     default_reps = excluded.default_reps;
 
--- name: UpdateExerciseSortOrder :exec
-UPDATE exercises SET sort_order = ? WHERE id = ?;
+-- name: ClearUserExerciseSortOrder :exec
+DELETE FROM user_exercise_sort_order WHERE user_id = ?;
+
+-- name: UpsertUserExerciseSortOrder :exec
+INSERT INTO user_exercise_sort_order (user_id, exercise_id, sort_order)
+VALUES (?, ?, ?)
+ON CONFLICT(user_id, exercise_id) DO UPDATE SET
+    sort_order = excluded.sort_order;
 
 -- =============================================================================
 -- USER EXERCISE WEIGHT
@@ -193,10 +209,15 @@ INSERT INTO sets (workout_id, exercise_id, set_index, target_reps, actual_reps,
 VALUES (?, ?, ?, ?, NULL, ?);
 
 -- name: UpdateSetActualReps :exec
-UPDATE sets SET actual_reps = ? WHERE id = ?;
+-- user_id check is defence in depth; handlers already verify ownership.
+UPDATE sets SET actual_reps = ?
+WHERE sets.id = ?
+  AND sets.workout_id IN (SELECT w.id FROM workouts w WHERE w.user_id = ?);
 
 -- name: UpdateSetsWeightForExercise :exec
-UPDATE sets SET weight_kg = ? WHERE workout_id = ? AND exercise_id = ?;
+UPDATE sets SET weight_kg = ?
+WHERE sets.workout_id = ? AND sets.exercise_id = ?
+  AND sets.workout_id IN (SELECT w.id FROM workouts w WHERE w.user_id = ?);
 
 -- =============================================================================
 -- WALKING SESSIONS
@@ -207,8 +228,16 @@ SELECT workout_id, duration_min, speed_x10, incline_x10
 FROM walking_sessions WHERE workout_id = ?;
 
 -- name: UpsertWalkingSession :exec
+-- The SELECT yields zero rows (and therefore writes nothing) if the workout
+-- does not belong to user_id. Handlers already check ownership; this is
+-- defence in depth.
 INSERT INTO walking_sessions (workout_id, duration_min, speed_x10, incline_x10)
-VALUES (?, ?, ?, ?)
+SELECT w.id,
+       sqlc.arg(duration_min),
+       sqlc.arg(speed_x10),
+       sqlc.arg(incline_x10)
+FROM workouts w
+WHERE w.id = sqlc.arg(workout_id) AND w.user_id = sqlc.arg(user_id)
 ON CONFLICT(workout_id) DO UPDATE SET
     duration_min = excluded.duration_min,
     speed_x10    = excluded.speed_x10,