feat: Implement atomic file uploads, enhance login rate limiting with IP+username, add CSRF protection to comment endpoints, and refine post title validation.

Author: zerox80

backend/src/handlers/auth.rs

@@ -24,7 +24,7 @@
 
 use crate::{security::{auth, csrf}, db::DbPool, models::*, repositories};
 use axum::{
-    extract::State,
+    extract::{ConnectInfo, State},
     http::{HeaderMap, StatusCode},
     Json,
 };
@@ -255,6 +255,7 @@ fn validate_password(password: &str) -> Result<(), String> {
 /// - Lockout countdown shown to user
 pub async fn login(
     State(pool): State<DbPool>,
+    ConnectInfo(addr): ConnectInfo<SocketAddr>,
     Json(payload): Json<LoginRequest>,
 ) -> Result<(HeaderMap, Json<LoginResponse>), (StatusCode, Json<ErrorResponse>)> {
     let username = payload.username.trim().to_string();
@@ -266,7 +267,11 @@ pub async fn login(
         return Err((StatusCode::BAD_REQUEST, Json(ErrorResponse { error: e })));
     }
 
-    let attempt_key = hash_login_identifier(&username);
+    // Rate limit based on (IP + Username) to prevent DoS against specific users
+    // If we only used username, an attacker could lock out 'admin' by spamming bad passwords.
+    // If we only used IP, an attacker could rotate IPs to brute force.
+    // Using both is a balanced approach.
+    let attempt_key = hash_login_identifier(&format!("{}:{}", addr.ip(), username));
 
     let attempt_record = repositories::users::get_login_attempt(&pool, &attempt_key)
         .await

backend/src/handlers/comments.rs

@@ -187,6 +187,7 @@ pub async fn create_comment(
     State(pool): State<DbPool>,
     ConnectInfo(addr): ConnectInfo<SocketAddr>,
     Path(tutorial_id): Path<String>,
+    _csrf: crate::security::csrf::CsrfGuard,
     Json(payload): Json<CreateCommentRequest>,
 ) -> Result<Json<Comment>, (StatusCode, Json<ErrorResponse>)> {
     if let Err(e) = validate_tutorial_id(&tutorial_id) {
@@ -294,6 +295,7 @@ pub async fn create_post_comment(
     ConnectInfo(addr): ConnectInfo<SocketAddr>,
     Path(post_id): Path<String>,
     auth::OptionalClaims(claims): auth::OptionalClaims,
+    _csrf: crate::security::csrf::CsrfGuard,
     Json(payload): Json<CreateCommentRequest>,
 ) -> Result<Json<Comment>, (StatusCode, Json<ErrorResponse>)> {
     // Verify post exists
@@ -356,7 +358,11 @@ async fn create_comment_internal(
                 }
 
                 // Enforce strict name validation (alphanumeric and spaces)
-                let name_regex = regex::Regex::new(r"^[a-zA-Z0-9 ]+$").unwrap();
+                static NAME_REGEX: std::sync::OnceLock<regex::Regex> = std::sync::OnceLock::new();
+                let name_regex = NAME_REGEX.get_or_init(|| {
+                    regex::Regex::new(r"^[a-zA-Z0-9 ]+$").unwrap()
+                });
+
                 if !name_regex.is_match(trimmed) {
                      return Err((
                         StatusCode::BAD_REQUEST,
@@ -476,6 +482,7 @@ pub async fn delete_comment(
     claims: auth::Claims,
     State(pool): State<DbPool>,
     Path(id): Path<String>,
+    _csrf: crate::security::csrf::CsrfGuard,
 ) -> Result<StatusCode, (StatusCode, Json<ErrorResponse>)> {
     // Fetch the comment first to check ownership
     let comment = repositories::comments::get_comment(&pool, &id)
@@ -554,6 +561,7 @@ pub async fn vote_comment(
     State(pool): State<DbPool>,
     claims: auth::Claims,
     Path(id): Path<String>,
+    _csrf: crate::security::csrf::CsrfGuard,
 ) -> Result<Json<Comment>, (StatusCode, Json<ErrorResponse>)> {
     // Check if comment exists
     let exists = repositories::comments::check_comment_exists(&pool, &id)
@@ -606,6 +614,16 @@ pub async fn vote_comment(
     repositories::comments::add_vote(&pool, &id, &voter_id)
         .await
         .map_err(|e| {
+            if let sqlx::Error::Database(db_err) = &e {
+                if db_err.is_unique_violation() {
+                    return (
+                        StatusCode::CONFLICT,
+                        Json(ErrorResponse {
+                            error: "You have already voted on this comment".to_string(),
+                        }),
+                    );
+                }
+            }
             tracing::error!("Database error recording vote: {}", e);
             (
                 StatusCode::INTERNAL_SERVER_ERROR,

backend/src/handlers/site_posts.rs

@@ -356,7 +356,16 @@ pub async fn update_post(
     }
 
     if let Some(ref title) = payload.title {
-        if title.trim().is_empty() || title.trim().len() > MAX_TITLE_LEN {
+        let trimmed = title.trim();
+        if trimmed.is_empty() {
+             return Err((
+                StatusCode::BAD_REQUEST,
+                Json(ErrorResponse {
+                    error: "Title cannot be empty".to_string(),
+                }),
+            ));
+        }
+        if trimmed.len() > MAX_TITLE_LEN {
             return Err((
                 StatusCode::BAD_REQUEST,
                 Json(ErrorResponse {
@@ -367,6 +376,9 @@ pub async fn update_post(
     }
 
     let mut payload = payload;
+    if let Some(title) = payload.title.as_mut() {
+        *title = title.trim().to_string();
+    }
     if let Some(slug) = payload.slug.as_mut() {
         *slug = sanitize_slug(slug);
     }

backend/src/handlers/tutorials/mod.rs

@@ -563,16 +563,7 @@ pub async fn update_tutorial(
         return Err((StatusCode::BAD_REQUEST, Json(ErrorResponse { error: e })));
     }
 
-    // Step 4: Handle version increment for optimistic concurrency control
-    let new_version = tutorial.version.checked_add(1).ok_or_else(|| {
-        tracing::error!("Tutorial version overflow for id: {}", id);
-        (
-            StatusCode::INTERNAL_SERVER_ERROR,
-            Json(ErrorResponse {
-                error: "Tutorial version overflow".to_string(),
-            }),
-        )
-    })?;
+
 
     // Step 5: Handle topics serialization
     let (topics_json, topics_vec) = if let Some(t) = payload.topics {
@@ -612,7 +603,7 @@ pub async fn update_tutorial(
     };
 
     // Step 6: Atomic Update operation in repository
-    // Note: The repository update should include a WHERE version = old_version check
+    // The repository handles the version increment and the WHERE version = current_version check
     let updated_tutorial = repositories::tutorials::update_tutorial(
         &pool,
         &id,
@@ -623,7 +614,7 @@ pub async fn update_tutorial(
         &color,
         &topics_json,
         &topics_vec,
-        new_version.try_into().unwrap_or(1),
+        tutorial.version, // Fix: Pass current version, not new_version
     )
     .await
     .map_err(|e| {

backend/src/handlers/upload.rs

@@ -144,12 +144,15 @@ pub async fn upload_image(
             }
 
             let filepath = upload_path_base.join(&new_filename);
+            // Create a temporary file path
+            let temp_filename = format!("{}.tmp", id);
+            let temp_filepath = upload_path_base.join(&temp_filename);
 
-            // Open the file for writing (using async tokio file)
-            let mut file = match tokio::fs::File::create(&filepath).await {
+            // Open the temp file for writing
+            let mut file = match tokio::fs::File::create(&temp_filepath).await {
                 Ok(file) => file,
                 Err(e) => {
-                    tracing::error!("Failed to create file {}: {}", filepath.display(), e);
+                    tracing::error!("Failed to create temp file {}: {}", temp_filepath.display(), e);
                     return Err((
                         StatusCode::INTERNAL_SERVER_ERROR,
                         Json(ErrorResponse {
@@ -161,10 +164,10 @@ pub async fn upload_image(
 
             use tokio::io::AsyncWriteExt; // Required for write_all and flush
 
-            // Write the first chunk (the one we peeked at for inference)
+            // Write the first chunk
             if let Err(e) = file.write_all(&first_chunk).await {
-                 tracing::error!("Failed to write first chunk to {}: {}", filepath.display(), e);
-                 let _ = tokio::fs::remove_file(&filepath).await; // Cleanup partially written file
+                 tracing::error!("Failed to write first chunk to {}: {}", temp_filepath.display(), e);
+                 let _ = tokio::fs::remove_file(&temp_filepath).await;
                  return Err((
                     StatusCode::INTERNAL_SERVER_ERROR,
                     Json(ErrorResponse {
@@ -181,7 +184,7 @@ pub async fn upload_image(
                     Ok(opt) => opt,
                     Err(err) => {
                         tracing::error!("Failed to read chunk: {}", err);
-                        let _ = tokio::fs::remove_file(&filepath).await; // Cleanup on network/parsing error
+                        let _ = tokio::fs::remove_file(&temp_filepath).await; 
                          return Err((
                             StatusCode::INTERNAL_SERVER_ERROR,
                             Json(ErrorResponse {
@@ -191,7 +194,6 @@ pub async fn upload_image(
                     }
                 };
 
-                // Check if we hit the end of the field
                 let chunk = match chunk_option {
                     Some(c) => c,
                     None => break,
@@ -200,7 +202,7 @@ pub async fn upload_image(
                 // ENFORCEMENT: Track total size to prevent Disk Space exhaustion (DoS)
                 total_size += chunk.len();
                 if total_size > MAX_FILE_SIZE {
-                    let _ = tokio::fs::remove_file(&filepath).await;
+                    let _ = tokio::fs::remove_file(&temp_filepath).await;
                     return Err((
                         StatusCode::BAD_REQUEST,
                         Json(ErrorResponse {
@@ -211,8 +213,8 @@ pub async fn upload_image(
 
                 // Write chunk to disk
                 if let Err(e) = file.write_all(&chunk).await {
-                     tracing::error!("Failed to write chunk to {}: {}", filepath.display(), e);
-                     let _ = tokio::fs::remove_file(&filepath).await; // Cleanup on disk error
+                     tracing::error!("Failed to write chunk to {}: {}", temp_filepath.display(), e);
+                     let _ = tokio::fs::remove_file(&temp_filepath).await;
                      return Err((
                         StatusCode::INTERNAL_SERVER_ERROR,
                         Json(ErrorResponse {
@@ -224,8 +226,8 @@ pub async fn upload_image(
 
             // Sync buffers to disk
             if let Err(e) = file.flush().await {
-                 tracing::error!("Failed to flush file {}: {}", filepath.display(), e);
-                 let _ = tokio::fs::remove_file(&filepath).await;
+                 tracing::error!("Failed to flush file {}: {}", temp_filepath.display(), e);
+                 let _ = tokio::fs::remove_file(&temp_filepath).await;
                  return Err((
                     StatusCode::INTERNAL_SERVER_ERROR,
                     Json(ErrorResponse {
@@ -234,6 +236,18 @@ pub async fn upload_image(
                 ));
             }
 
+            // Atomic rename from temp to final
+            if let Err(e) = tokio::fs::rename(&temp_filepath, &filepath).await {
+                tracing::error!("Failed to rename temp file {} to {}: {}", temp_filepath.display(), filepath.display(), e);
+                let _ = tokio::fs::remove_file(&temp_filepath).await;
+                return Err((
+                    StatusCode::INTERNAL_SERVER_ERROR,
+                    Json(ErrorResponse {
+                        error: "Failed to save file".to_string(),
+                    }),
+                ));
+            }
+
             // SUCCESS path
             tracing::info!("Successfully uploaded image: {}", filepath.display());