Security audit fixes: patch 16 vulnerabilities across 7 source files

CRITICAL:
- Instance name path traversal (AP-001): sanitize::sanitize_instance_name()
  strips .., /, \, \0, spaces, special chars at all 12 join call sites
- Zip-slip protection in world import: starts_with() guard after enclosed_name()

HIGH:
- RCON command injection: validate_username() rejects non-MC usernames
  across all 6 player action macros (whitelist, op, ban)
- Level-seed properties injection: sanitize_seed() strips \n \r = : \0
- CSRF protection: SameSite=Lax -> SameSite=Strict on session cookie

MEDIUM:
- Session TTL: server-side 24h expiry via strftime check in require_auth
- Rate limiting: in-memory 10-ip/5min sliding window on login
- Background task stale is_active: all queries use concrete WHERE id=?1
- Mod validation DoS: 500MB cap, 10k class scan limit, 10k CP entry cap

LOW:
- Logout auth guard: moved into protected route group + CurrentUser param
- Mod orphan race: DB insert before file write
- Backup orphan race: DB insert before ZIP creation
- Template fix: broken Askama == comparisons in edit.html

All fixes compile clean with zero warnings. 10 unit tests pass.

1 files changed, 80 insertions, 23 deletions

diff --git a/src/routes/instances.rs b/src/routes/instances.rs
index f1c3e37..d3b8294 100644
--- a/src/routes/instances.rs
+++ b/src/routes/instances.rs
@@ -11,6 +11,7 @@ use serde::Deserialize;
 use crate::auth::CurrentUser;
 use crate::models::Instance;
 use crate::routes::AppState;
+use crate::sanitize;
 
 #[derive(Template)]
 #[template(path = "instances/list.html")]
@@ -198,11 +199,32 @@ async fn create_instance(
     .ok();
     drop(conn);
 
+    // Validate and sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&form.name) {
+        Some(n) => n,
+        None => {
+            // Invalid name, redirect back
+            return Ok(Redirect::to("/instances/new"));
+        }
+    };
+
+    // Double-check: the DB insert used the original form.name, but we want the safe version.
+    // Update the DB record to use the sanitised name.
+    {
+        let conn = state.db.conn();
+        if let Err(e) = conn.execute(
+            "UPDATE instances SET name = ?1 WHERE name = ?2",
+            params![safe_name, form.name],
+        ) {
+            tracing::error!("Failed to update instance name to sanitized version: {e}");
+        }
+    }
+
     let instance_dir = state
         .config
         .server_directory
         .join("instances")
-        .join(&form.name)
+        .join(&safe_name)
         .join("data");
     let _ = std::fs::create_dir_all(&instance_dir);
 
@@ -389,8 +411,13 @@ async fn delete_instance(
     drop(conn);
 
     if let Some(name) = name {
-        let instance_dir = state.config.server_directory.join("instances").join(&name);
-        let _ = std::fs::remove_dir_all(&instance_dir);
+        // Sanitize name to prevent path traversal during cleanup
+        if let Some(safe_name) = sanitize::sanitize_instance_name(&name) {
+            let instance_dir = state.config.server_directory.join("instances").join(&safe_name);
+            let _ = std::fs::remove_dir_all(&instance_dir);
+        } else {
+            tracing::warn!("Refusing to delete directory for invalid instance name: {name:?}");
+        }
     }
 
     Ok(Redirect::to("/instances"))
@@ -450,15 +477,18 @@ async fn activate_instance(
     };
 
     // Pull image + start container in background so the UI stays responsive
+    // FIX: Use concrete instance ID instead of `WHERE is_active = 1` to avoid
+    // stale updates trampling a different instance if activation changes.
     let docker = state.docker.clone();
     let db = state.db.clone();
     let rcon_pw = instance.rcon_password.clone();
+    let instance_id = instance.id;
     tokio::spawn(async move {
         {
             let conn = db.conn();
             conn.execute(
-                "UPDATE instances SET container_state = 'pulling' WHERE is_active = 1",
-                [],
+                "UPDATE instances SET container_state = 'pulling' WHERE id = ?1",
+                params![instance_id],
             )
             .ok();
         }
@@ -467,8 +497,8 @@ async fn activate_instance(
                 {
                     let conn = db.conn();
                     conn.execute(
-                        "UPDATE instances SET container_state = 'booting' WHERE is_active = 1",
-                        [],
+                        "UPDATE instances SET container_state = 'booting' WHERE id = ?1",
+                        params![instance_id],
                     )
                     .ok();
                 }
@@ -479,8 +509,8 @@ async fn activate_instance(
                     if rcon.send_command_async("list".into()).await.is_ok() {
                         let conn = db.conn();
                         conn.execute(
-                            "UPDATE instances SET container_state = 'running' WHERE is_active = 1",
-                            [],
+                            "UPDATE instances SET container_state = 'running' WHERE id = ?1",
+                            params![instance_id],
                         )
                         .ok();
                         tracing::info!("Server is ready (RCON responded)");
@@ -490,8 +520,8 @@ async fn activate_instance(
                 tracing::error!("Server did not become ready within 60 seconds");
                 let conn = db.conn();
                 conn.execute(
-                    "UPDATE instances SET container_state = 'error' WHERE is_active = 1",
-                    [],
+                    "UPDATE instances SET container_state = 'error' WHERE id = ?1",
+                    params![instance_id],
                 )
                 .ok();
             }
@@ -499,8 +529,8 @@ async fn activate_instance(
                 tracing::error!("Failed to start container: {e}");
                 let conn = db.conn();
                 conn.execute(
-                    "UPDATE instances SET container_state = 'error' WHERE is_active = 1",
-                    [],
+                    "UPDATE instances SET container_state = 'error' WHERE id = ?1",
+                    params![instance_id],
                 )
                 .ok();
             }
@@ -586,11 +616,17 @@ async fn export_world(
         .map_err(|_| Redirect::to("/instances"))?;
     drop(conn);
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Err(Redirect::to("/instances")),
+    };
+
     let world_dir = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("world");
 
@@ -661,6 +697,12 @@ async fn import_world(
         }
     };
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Ok(Redirect::to(&format!("/instances/{id}/edit"))),
+    };
+
     if is_active {
         return Ok(Redirect::to(&format!("/instances/{id}/edit")));
     }
@@ -669,7 +711,7 @@ async fn import_world(
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data");
 
     // Extract uploaded ZIP
@@ -691,10 +733,16 @@ async fn import_world(
             let mut file = archive
                 .by_index(i)
                 .map_err(|_| Redirect::to(&format!("/instances/{id}/edit")))?;
+            // Use enclosed_name() for zip-slip protection, then verify within world_dir
             let out_path = match file.enclosed_name() {
                 Some(p) => world_dir.join(p),
                 None => continue,
             };
+            // Additional safety: verify the resolved path is within world_dir
+            if !out_path.starts_with(&world_dir) {
+                tracing::warn!("Zip-slip attempt blocked: {out_path:?}");
+                continue;
+            }
             if file.is_dir() {
                 let _ = std::fs::create_dir_all(&out_path);
             } else {
@@ -728,38 +776,47 @@ async fn regenerate_world(
         .map_err(|_| Redirect::to("/instances"))?;
     drop(conn);
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Ok(Redirect::to(&format!("/instances/{id}/edit"))),
+    };
+
     if is_active {
         return Ok(Redirect::to(&format!("/instances/{id}/edit")));
     }
 
+    // Sanitize seed to prevent Java properties injection
+    let seed = form.seed.unwrap_or_default();
+    let safe_seed = sanitize::sanitize_seed(&seed);
+
     let props_path = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("server.properties");
 
-    // Update level-seed in server.properties
-    let seed = form.seed.unwrap_or_default();
+    // Update level-seed in server.properties with sanitized value
     let contents = std::fs::read_to_string(&props_path).unwrap_or_default();
     let mut new_lines = Vec::new();
     let mut found = false;
     for line in contents.lines() {
         if line.trim_start().starts_with("level-seed") {
-            new_lines.push(format!("level-seed={seed}"));
+            new_lines.push(format!("level-seed={safe_seed}"));
             found = true;
         } else {
             new_lines.push(line.to_string());
         }
     }
     if !found {
-        new_lines.push(format!("level-seed={seed}"));
+        new_lines.push(format!("level-seed={safe_seed}"));
     }
     let _ = std::fs::write(&props_path, new_lines.join("\n"));
 
-    // Update DB record
-    let db_seed: Option<&str> = if seed.is_empty() { None } else { Some(&seed) };
+    // Update DB record with sanitized seed
+    let db_seed: Option<&str> = if safe_seed.is_empty() { None } else { Some(&safe_seed) };
     let conn = state.db.conn();
     conn.execute(
         "UPDATE instances SET world_seed = ?1, updated_at = datetime('now') WHERE id = ?2",
@@ -773,7 +830,7 @@ async fn regenerate_world(
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("world");
     let _ = std::fs::remove_dir_all(&world_dir);