fix: resolve 7 critical security findings from code review

CR-1: Path traversal in createReceiptZip — validate image_path is within storage/
CR-2: Missing authz on EditExpense/UpdateExpense — verify event ownership
CR-3: OTP timing side-channel — use crypto/subtle.ConstantTimeCompare
CR-4: Logout doesn't invalidate session — moved to AuthHandler with Sessions.Delete()
CR-5: OTP reuse race condition — mutex lock around validate+delete
CR-6: Live credentials on disk — removed .env from disk entirely
CR-7: No TLS — documented as expected behind-proxy deployment

Additional:
- Removed stale github.com/expenseflow import path from auth.go
- Made EnvironmentFile optional (prefix with -) so .env is not required
- App runs and starts clean without any .env file

contrib/receiptnext.service

@@ -11,7 +11,7 @@ WorkingDirectory=/opt/receiptnext
 ExecStart=/opt/receiptnext/app
 Restart=always
 RestartSec=5
-EnvironmentFile=/opt/receiptnext/.env
+EnvironmentFile=-/opt/receiptnext/.env
 StandardOutput=append:/var/log/receiptnext.log
 StandardError=append:/var/log/receiptnext.log
 

install.sh

@@ -295,7 +295,7 @@ WorkingDirectory=${INSTALL_DIR}
 ExecStart=${INSTALL_DIR}/app
 Restart=always
 RestartSec=5
-EnvironmentFile=${INSTALL_DIR}/.env
+EnvironmentFile=-${INSTALL_DIR}/.env
 StandardOutput=append:/var/log/receiptnext.log
 StandardError=append:/var/log/receiptnext.log
 

internal/auth/otp.go

@@ -4,6 +4,7 @@ package auth
 
 import (
 	"crypto/rand"
+	"crypto/subtle"
 	"fmt"
 	"sync"
 	"time"
@@ -33,7 +34,8 @@ func ValidateOTP(provided, stored string, expiresAt time.Time) bool {
 	if time.Now().After(expiresAt) {
 		return false
 	}
-	return provided == stored
+	// Use constant-time comparison to prevent timing side-channel attacks.
+	return subtle.ConstantTimeCompare([]byte(provided), []byte(stored)) == 1
 }
 
 // attemptData stores the failure count and timestamp for a single email.

internal/handlers/auth.go

@@ -10,6 +10,7 @@ import (
 	"log"
 	"net/http"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/cclohmar/ReceiptNext/internal/auth"
@@ -35,6 +36,8 @@ type AuthHandler struct {
 	Sessions       *auth.SessionStore
 	FailureTracker *auth.FailureTracker
 	EmailSender    *email.Sender
+
+	otpMu sync.Mutex // prevents OTP reuse via race conditions
 }
 
 // ---------------------------------------------------------------------------
@@ -164,19 +167,21 @@ func (h *AuthHandler) VerifyOTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Validate the OTP code and expiry.
+	// Validate + delete OTP atomically to prevent race-condition reuse.
+	h.otpMu.Lock()
 	if !auth.ValidateOTP(otpCode, stored.OTPCode, expiresAt) {
+		h.otpMu.Unlock()
 		h.FailureTracker.RecordFailure(emailAddr)
 		renderOTPForm(w, emailAddr, "Invalid or expired OTP code. Please try again.")
 		return
 	}
 
-	// Successful verification: clean up and create session.
+	// Successful verification: delete OTP immediately (still under lock).
 	h.FailureTracker.Reset(emailAddr)
 	if err := database.DeleteOTP(h.DB, emailAddr); err != nil {
 		log.Printf("ERROR [%s] handlers: VerifyOTP DeleteOTP(%s): %v", time.Now().Format(time.RFC3339), emailAddr, err)
-		// Non-fatal — the OTP is already validated.
 	}
+	h.otpMu.Unlock()
 
 	// Retrieve the user record to obtain the user ID.
 	user, err := database.GetUserByEmail(h.DB, emailAddr)
@@ -292,6 +297,25 @@ func renderOTPForm(w http.ResponseWriter, email string, errMsg string) {
 // collectOTP reads the 6 individual digit form values and concatenates them
 // into a single 6-character OTP code string. Returns an empty string if any
 // digit is missing.
+// Logout clears the session cookie and invalidates the server-side session.
+func (h *AuthHandler) Logout(w http.ResponseWriter, r *http.Request) {
+	// Invalidate the server-side session.
+	if cookie, err := r.Cookie("session_token"); err == nil && cookie.Value != "" {
+		h.Sessions.Delete(cookie.Value)
+	}
+	// Clear the cookie on the client side.
+	http.SetCookie(w, &http.Cookie{
+		Name:     "session_token",
+		Value:    "",
+		Path:     "/",
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+		MaxAge:   -1,
+	})
+	w.Header().Set("HX-Redirect", "/")
+	w.WriteHeader(http.StatusOK)
+}
+
 func collectOTP(r *http.Request) string {
 	var b strings.Builder
 	for i := 0; i < 6; i++ {

internal/handlers/expenses.go

@@ -359,6 +359,13 @@ func (h *ExpenseHandler) EditExpense(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Verify ownership: the expense's event must belong to the current user.
+	event, err := database.GetEventByID(h.DB, expense.EventID)
+	if err != nil || event == nil || event.UserID != getUserID(r) {
+		http.Error(w, "Forbidden", http.StatusForbidden)
+		return
+	}
+
 	tmpl, err := template.ParseFiles("templates/receipt_form.html")
 	if err != nil {
 		log.Printf("ERROR [%s] handlers: EditExpense: parse template: %v",
@@ -426,6 +433,13 @@ func (h *ExpenseHandler) UpdateExpense(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Verify ownership: the expense's event must belong to the current user.
+	event, err := database.GetEventByID(h.DB, existing.EventID)
+	if err != nil || event == nil || event.UserID != getUserID(r) {
+		http.Error(w, "Forbidden", http.StatusForbidden)
+		return
+	}
+
 	expense := database.Expense{
 		ID:              expenseID,
 		EventID:         existing.EventID,

internal/handlers/file.go

@@ -364,11 +364,19 @@ func createReceiptZip(eventName string, expenses []database.Expense) (*email.Att
 			continue
 		}
 
+		// Prevent path traversal — only allow files within the storage directory.
+		cleanPath := filepath.Clean(exp.ImagePath)
+		if !strings.HasPrefix(cleanPath, "storage") && !strings.HasPrefix(cleanPath, "./storage") {
+			log.Printf("WARN  [%s] handlers: createReceiptZip: blocked path traversal attempt: %q",
+				time.Now().Format(time.RFC3339), exp.ImagePath)
+			continue
+		}
+
 		// Read the image file from disk.
-		data, err := os.ReadFile(exp.ImagePath)
+		data, err := os.ReadFile(cleanPath)
 		if err != nil {
 			log.Printf("WARN  [%s] handlers: createReceiptZip: reading %q: %v",
-				time.Now().Format(time.RFC3339), exp.ImagePath, err)
+				time.Now().Format(time.RFC3339), cleanPath, err)
 			continue
 		}
 

main.go

@@ -156,20 +156,9 @@ func main() {
 	r.Post("/request-otp", authHandler.RequestOTP)
 	r.Post("/verify-otp", authHandler.VerifyOTP)
 
-	// ---- Logout ----
+	// ---- Logout (invalidates server-side session + clears cookie) ----
 
-	r.Post("/logout", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	r.Post("/logout", authHandler.Logout)
-		// Delete the session cookie.
-		http.SetCookie(w, &http.Cookie{
-			Name:     "session_token",
-			Value:    "",
-			Path:     "/",
-			HttpOnly: true,
-			MaxAge:   -1,
-		})
-		w.Header().Set("HX-Redirect", "/")
-		w.WriteHeader(http.StatusOK)
-	}))
 
 	// ---- Protected routes (auth required) ----