fix: harden RBAC access controls

src/db/rbac.rs

@@ -478,24 +478,54 @@ pub enum DeleteRoleResult {
     HasUsers(i64),
 }
 
-/// Idempotently ensure the admin role has all permissions from Permission::ALL.
+/// Permissions that the moderator role should have.
+/// Everything except admin-only permissions (UsersManage, SettingsManage, NotesManage).
+const MODERATOR_PERMISSIONS: &[Permission] = &[
+    Permission::ModsInstall,
+    Permission::ModsUpdate,
+    Permission::ModsRemove,
+    Permission::ModsDisable,
+    Permission::ModsConfigEdit,
+    Permission::ConvoyManage,
+    Permission::SvmEdit,
+    Permission::RequestsResolve,
+    Permission::ServerControl,
+    Permission::ServerLogs,
+    Permission::ServerMetrics,
+    Permission::HeadlessManage,
+    Permission::QueueManage,
+    Permission::ItemsGive,
+    Permission::NotesEdit,
+];
+
+/// Idempotently ensure built-in roles have their expected permissions.
 /// Called after migrations to pick up newly added permissions on upgrade.
 /// Additive only — never removes permissions.
 pub fn sync_builtin_role_permissions(conn: &Connection) -> rusqlite::Result<()> {
-    let admin_id: Option<i64> = conn
-        .query_row("SELECT id FROM roles WHERE name = 'admin'", [], |row| {
-            row.get(0)
-        })
-        .optional()?;
-
-    if let Some(id) = admin_id {
-        for perm in Permission::ALL {
-            conn.execute(
-                "INSERT OR IGNORE INTO role_permissions (role_id, permission) VALUES (?1, ?2)",
-                params![id, perm.as_str()],
-            )?;
+    // ponytail: single query per role, INSERT OR IGNORE is idempotent
+    let sync_role = |role_name: &str, perms: &[Permission]| -> rusqlite::Result<()> {
+        let role_id: Option<i64> = conn
+            .query_row(
+                "SELECT id FROM roles WHERE name = ?1",
+                params![role_name],
+                |row| row.get(0),
+            )
+            .optional()?;
+
+        if let Some(id) = role_id {
+            for perm in perms {
+                conn.execute(
+                    "INSERT OR IGNORE INTO role_permissions (role_id, permission) VALUES (?1, ?2)",
+                    params![id, perm.as_str()],
+                )?;
+            }
         }
-    }
+        Ok(())
+    };
+
+    sync_role("admin", Permission::ALL)?;
+    sync_role("moderator", MODERATOR_PERMISSIONS)?;
+    // player intentionally has no permissions
     Ok(())
 }
 
@@ -534,10 +564,15 @@ mod tests {
     fn moderator_permissions() {
         let db = Database::open_in_memory().unwrap();
         let perms = db.get_permissions_for_role("moderator").unwrap();
-        assert!(perms.contains(&Permission::ModsInstall));
-        assert!(perms.contains(&Permission::ServerControl));
+        // Moderator should have exactly the MODERATOR_PERMISSIONS set
+        assert_eq!(perms.len(), MODERATOR_PERMISSIONS.len());
+        for p in MODERATOR_PERMISSIONS {
+            assert!(perms.contains(p), "moderator missing {:?}", p);
+        }
+        // Admin-only permissions must be absent
         assert!(!perms.contains(&Permission::UsersManage));
         assert!(!perms.contains(&Permission::SettingsManage));
+        assert!(!perms.contains(&Permission::NotesManage));
     }
 
     #[test]
@@ -670,4 +705,22 @@ mod tests {
         assert_eq!(perms_after_sync.len(), Permission::ALL.len());
         assert!(perms_after_sync.contains(&Permission::SettingsManage));
     }
+
+    #[test]
+    fn sync_adds_new_moderator_permissions() {
+        let db = Database::open_in_memory().unwrap();
+        // Simulate a pre-upgrade state: remove a permission that sync should restore
+        db.conn().execute(
+            "DELETE FROM role_permissions WHERE role_id = (SELECT id FROM roles WHERE name = 'moderator') AND permission = 'items.give'",
+            [],
+        ).unwrap();
+        let perms = db.get_permissions_for_role("moderator").unwrap();
+        assert!(!perms.contains(&Permission::ItemsGive));
+
+        sync_builtin_role_permissions(db.conn()).unwrap();
+        let perms = db.get_permissions_for_role("moderator").unwrap();
+        assert!(perms.contains(&Permission::ItemsGive));
+        // Still no admin-only permissions
+        assert!(!perms.contains(&Permission::UsersManage));
+    }
 }

src/web/auth.rs

@@ -99,6 +99,17 @@ pub fn require_permission(
     Ok(())
 }
 
+/// Require that the requesting user is viewing their own data or has UsersManage.
+pub fn require_self_or_admin(
+    user: &SessionUser,
+    target_username: &str,
+) -> std::result::Result<(), WebError> {
+    if user.username != target_username && !user.has_permission(Permission::UsersManage) {
+        return Err(WebError::Forbidden);
+    }
+    Ok(())
+}
+
 pub fn set_session_user(session: &Session, user: &SessionUser) -> Result<()> {
     session
         .insert("user_id", user.user_id)
@@ -313,6 +324,34 @@ mod tests {
         assert!(!user.can("nonexistent.perm"));
     }
 
+    #[test]
+    fn require_self_or_admin_allows_own_data() {
+        let user = SessionUser {
+            user_id: 1,
+            username: "alice".into(),
+            role_name: "player".into(),
+            role_display_name: "Player".into(),
+            permissions: HashSet::new(),
+            has_password: true,
+        };
+        assert!(require_self_or_admin(&user, "alice").is_ok());
+        assert!(require_self_or_admin(&user, "bob").is_err());
+    }
+
+    #[test]
+    fn require_self_or_admin_allows_users_manage() {
+        let user = SessionUser {
+            user_id: 1,
+            username: "admin".into(),
+            role_name: "admin".into(),
+            role_display_name: "Admin".into(),
+            permissions: HashSet::from([Permission::UsersManage]),
+            has_password: true,
+        };
+        assert!(require_self_or_admin(&user, "alice").is_ok());
+        assert!(require_self_or_admin(&user, "bob").is_ok());
+    }
+
     #[test]
     fn password_complexity_validation() {
         // Too short

src/web/handlers/mods.rs

@@ -3348,7 +3348,7 @@ pub async fn groups_partial(
     session: Session,
 ) -> actix_web::Result<HttpResponse> {
     let user = require_auth(&req)?;
-    require_permission(&user, Permission::ModsInstall)?;
+    require_permission(&user, Permission::ConvoyManage)?;
     let csrf_token = crate::web::csrf::get_or_create_token(&session);
 
     let html = render_groups_tab(&state, &csrf_token).await?;
@@ -3370,7 +3370,7 @@ pub async fn new_group_card(
     query: Query<NewGroupQuery>,
 ) -> actix_web::Result<HttpResponse> {
     let user = require_auth(&req)?;
-    require_permission(&user, Permission::ModsInstall)?;
+    require_permission(&user, Permission::ConvoyManage)?;
     let _csrf_token = crate::web::csrf::get_or_create_token(&session);
 
     let all_mods = fetch_mods_with_client_files(&state).await?;
@@ -3447,7 +3447,7 @@ pub async fn save_groups(
     body: web::Json<SaveGroupsRequest>,
 ) -> actix_web::Result<HttpResponse> {
     let user = require_auth(&req)?;
-    require_permission(&user, Permission::ModsInstall)?;
+    require_permission(&user, Permission::ConvoyManage)?;
 
     if !crate::web::csrf::validate_token(&session, &body.csrf_token) {
         return Err(WebError::Forbidden.into());

src/web/handlers/profiles.rs

@@ -7,7 +7,7 @@ use askama::Template;
 use serde::Deserialize;
 
 use crate::spt::profiles::{load_profile_detail, load_stash_items, ProfileDetail, QuestState};
-use crate::web::auth::require_auth;
+use crate::web::auth::{require_auth, require_self_or_admin};
 use crate::web::csrf;
 use crate::web::error::WebError;
 use crate::web::flash::{take_flash, FlashMessage};
@@ -125,6 +125,7 @@ pub async fn profile_page(
     let flash = take_flash(&session);
     let csrf_token = csrf::get_or_create_token(&session);
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
 
     let db = state.db.clone();
     let dirs = Arc::clone(&state.dirs);
@@ -252,8 +253,9 @@ pub async fn quests_partial(
     path: Path<String>,
     query: Query<QuestsQuery>,
 ) -> actix_web::Result<Html> {
-    require_auth(&req)?;
+    let user = require_auth(&req)?;
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
     let detail = load_detail_for_user(&state, &profile_username).await?;
 
     let total = detail.quests.len();
@@ -306,8 +308,9 @@ pub async fn traders_partial(
     req: HttpRequest,
     path: Path<String>,
 ) -> actix_web::Result<Html> {
-    require_auth(&req)?;
+    let user = require_auth(&req)?;
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
     let detail = load_detail_for_user(&state, &profile_username).await?;
 
     let trader_displays: Vec<TraderDisplay> = detail
@@ -338,8 +341,9 @@ pub async fn hideout_partial(
     req: HttpRequest,
     path: Path<String>,
 ) -> actix_web::Result<Html> {
-    require_auth(&req)?;
+    let user = require_auth(&req)?;
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
     let detail = load_detail_for_user(&state, &profile_username).await?;
 
     let area_displays: Vec<HideoutAreaDisplay> = detail
@@ -405,8 +409,9 @@ pub async fn stash_partial(
     path: Path<String>,
     query: Query<StashQuery>,
 ) -> actix_web::Result<Html> {
-    require_auth(&req)?;
+    let user = require_auth(&req)?;
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
 
     let db = state.db.clone();
     let lookup_username = profile_username.clone();

src/web/handlers/raids.rs

@@ -5,7 +5,7 @@ use askama::Template;
 use serde::Deserialize;
 
 use crate::db::raids::{LeaderboardEntry, Raid, RaidKill, ServerRaidStats, UserRaidStats};
-use crate::web::auth::require_auth;
+use crate::web::auth::{require_auth, require_self_or_admin};
 use crate::web::csrf;
 use crate::web::error::WebError;
 use crate::web::flash::{take_flash, FlashMessage};
@@ -175,6 +175,7 @@ pub async fn player_raids_page(
     let flash = take_flash(&session);
     let csrf_token = csrf::get_or_create_token(&session);
     let profile_username = path.into_inner();
+    require_self_or_admin(&user, &profile_username)?;
     let offset = query.offset.unwrap_or(0);
 
     let db = state.db.clone();
@@ -250,6 +251,8 @@ pub async fn raid_detail_page(
     .map_err(WebError::from)?
     .map_err(|_| WebError::NotFound)?;
 
+    require_self_or_admin(&user, &raid_username)?;
+
     let tmpl = RaidDetailPageTemplate {
         user,
         flash,