max / makenotwork
4 files changed,
+108 insertions,
-21 deletions
| @@ -137,6 +137,17 @@ pub(crate) async fn require_owner( | |||
| 137 | 137 | } | |
| 138 | 138 | ||
| 139 | 139 | /// Helper: fetch community + verify mod_or_owner role, returning 403 if not. | |
| 140 | + | /// | |
| 141 | + | /// Also rejects a suspended community (403). A suspended community is frozen to | |
| 142 | + | /// its own owner and mods: this gate fronts every mod-mutation handler | |
| 143 | + | /// (ban/unban/mute/unmute, flag dismiss/remove) plus the moderation page views, | |
| 144 | + | /// so folding the suspension check in here — rather than per handler — means a | |
| 145 | + | /// newly-added moderation route can't reinstate the drift the fuzz found (the | |
| 146 | + | /// 403 previously lived only on the settings/moderation *GET* pages while every | |
| 147 | + | /// write handler gated on role alone). The platform admin acts on a suspended | |
| 148 | + | /// community through the `_admin` routes / `require_mod_or_superadmin`, never | |
| 149 | + | /// this helper (a non-member admin's role is `None` and fails `is_mod_or_owner` | |
| 150 | + | /// regardless), so nothing here can lock the admin out. | |
| 140 | 151 | #[tracing::instrument(skip_all)] | |
| 141 | 152 | pub(crate) async fn require_mod_or_owner( | |
| 142 | 153 | state: &AppState, | |
| @@ -148,6 +159,9 @@ pub(crate) async fn require_mod_or_owner( | |||
| 148 | 159 | if !is_mod_or_owner(&role) { | |
| 149 | 160 | return Err((StatusCode::FORBIDDEN, "Forbidden").into_response()); | |
| 150 | 161 | } | |
| 162 | + | if community.suspended_at.is_some() { | |
| 163 | + | return Err((StatusCode::FORBIDDEN, "This community has been suspended.").into_response()); | |
| 164 | + | } | |
| 151 | 165 | Ok((community, role)) | |
| 152 | 166 | } | |
| 153 | 167 |
| @@ -31,11 +31,7 @@ pub(super) async fn pin_thread_handler( | |||
| 31 | 31 | ) -> Result<Redirect, Response> { | |
| 32 | 32 | let scope = | |
| 33 | 33 | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 34 | - | let role = scope.role(&state.db, user.user_id).await?; | |
| 35 | - | ||
| 36 | - | if !is_mod_or_owner(&role) { | |
| 37 | - | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 38 | - | } | |
| 34 | + | scope.require_mod_write(&state.db, user.user_id).await?; | |
| 39 | 35 | let thread_data = scope.resource; | |
| 40 | 36 | ||
| 41 | 37 | let new_pinned = !thread_data.pinned; | |
| @@ -65,11 +61,7 @@ pub(super) async fn lock_thread_handler( | |||
| 65 | 61 | ) -> Result<Redirect, Response> { | |
| 66 | 62 | let scope = | |
| 67 | 63 | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 68 | - | let role = scope.role(&state.db, user.user_id).await?; | |
| 69 | - | ||
| 70 | - | if !is_mod_or_owner(&role) { | |
| 71 | - | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 72 | - | } | |
| 64 | + | scope.require_mod_write(&state.db, user.user_id).await?; | |
| 73 | 65 | let thread_data = scope.resource; | |
| 74 | 66 | ||
| 75 | 67 | let new_locked = !thread_data.locked; | |
| @@ -102,11 +94,7 @@ pub(super) async fn mod_remove_post_handler( | |||
| 102 | 94 | RequireUser(user): RequireUser, | |
| 103 | 95 | ) -> Result<Redirect, Response> { | |
| 104 | 96 | let scope = CommunityScope::<PostForEdit>::resolve(&state.db, &slug, &post_id_str).await?; | |
| 105 | - | let role = scope.role(&state.db, user.user_id).await?; | |
| 106 | - | ||
| 107 | - | if !is_mod_or_owner(&role) { | |
| 108 | - | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 109 | - | } | |
| 97 | + | scope.require_mod_write(&state.db, user.user_id).await?; | |
| 110 | 98 | let post_data = scope.resource; | |
| 111 | 99 | let post_id = post_data.id; | |
| 112 | 100 | ||
| @@ -158,12 +146,9 @@ pub(super) async fn moderation_page( | |||
| 158 | 146 | RequireUser(user): RequireUser, | |
| 159 | 147 | ) -> Result<impl IntoResponse, Response> { | |
| 160 | 148 | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 149 | + | // `require_mod_or_owner` already 403s a suspended community. | |
| 161 | 150 | let (community, role) = require_mod_or_owner(&state, &slug, &user).await?; | |
| 162 | 151 | ||
| 163 | - | if community.suspended_at.is_some() { | |
| 164 | - | return Err((StatusCode::FORBIDDEN, "This community has been suspended.").into_response()); | |
| 165 | - | } | |
| 166 | - | ||
| 167 | 152 | // Opportunistic cleanup of expired bans/mutes | |
| 168 | 153 | if let Err(e) = mt_db::mutations::cleanup_expired_bans(&state.db, community.id).await { | |
| 169 | 154 | tracing::error!(error = %e, "failed to clean up expired bans"); |
| @@ -29,13 +29,13 @@ | |||
| 29 | 29 | //! loads a community resource without the equality check no longer compiles | |
| 30 | 30 | //! under the `-D warnings` gate. | |
| 31 | 31 | ||
| 32 | - | use axum::response::Response; | |
| 32 | + | use axum::response::{IntoResponse, Response}; | |
| 33 | 33 | use uuid::Uuid; | |
| 34 | 34 | ||
| 35 | 35 | use mt_core::types::CommunityRole; | |
| 36 | 36 | use mt_db::queries::{CommunityRow, PostForEdit, ThreadWithBreadcrumb}; | |
| 37 | 37 | ||
| 38 | - | use super::{check_write_access, db_error, get_community, get_role, parse_uuid}; | |
| 38 | + | use super::{check_write_access, db_error, get_community, get_role, is_mod_or_owner, parse_uuid}; | |
| 39 | 39 | ||
| 40 | 40 | /// A resource that lives inside a community and can be loaded by id. | |
| 41 | 41 | /// | |
| @@ -126,4 +126,33 @@ impl<T: ScopedResource> CommunityScope<T> { | |||
| 126 | 126 | ) -> Result<Option<CommunityRole>, Response> { | |
| 127 | 127 | get_role(db, user_id, self.community.id).await | |
| 128 | 128 | } | |
| 129 | + | ||
| 130 | + | /// Gate a moderation *mutation* on a scoped resource: the caller must be a | |
| 131 | + | /// mod or owner AND the community must not be suspended. Returns the role so | |
| 132 | + | /// callers that need it for finer checks (e.g. mod-vs-owner) don't re-query. | |
| 133 | + | /// | |
| 134 | + | /// This is the sealed equivalent of [`require_mod_or_owner`] for the | |
| 135 | + | /// resource-scoped handlers (pin/lock/mod-remove): folding the suspension | |
| 136 | + | /// check in beside the role check means a suspended community is frozen to | |
| 137 | + | /// its own mods here too, and a new scoped moderation handler can't drift | |
| 138 | + | /// back to a role-only gate. | |
| 139 | + | #[allow(clippy::result_large_err)] | |
| 140 | + | pub(crate) async fn require_mod_write( | |
| 141 | + | &self, | |
| 142 | + | db: &sqlx::PgPool, | |
| 143 | + | user_id: Uuid, | |
| 144 | + | ) -> Result<Option<CommunityRole>, Response> { | |
| 145 | + | let role = self.role(db, user_id).await?; | |
| 146 | + | if !is_mod_or_owner(&role) { | |
| 147 | + | return Err(axum::http::StatusCode::FORBIDDEN.into_response()); | |
| 148 | + | } | |
| 149 | + | if self.community.suspended_at.is_some() { | |
| 150 | + | return Err(( | |
| 151 | + | axum::http::StatusCode::FORBIDDEN, | |
| 152 | + | "This community has been suspended.", | |
| 153 | + | ) | |
| 154 | + | .into_response()); | |
| 155 | + | } | |
| 156 | + | Ok(role) | |
| 157 | + | } | |
| 129 | 158 | } |
| @@ -151,6 +151,65 @@ async fn mod_can_ban_member(_pool: sqlx::PgPool) { | |||
| 151 | 151 | assert!(resp.status.is_redirection() || resp.status == StatusCode::OK); | |
| 152 | 152 | } | |
| 153 | 153 | ||
| 154 | + | /// Regression (fuzz-2026-07-06 SERIOUS #2 — suspended-community mutation gap): | |
| 155 | + | /// a platform-suspended community is frozen to its own owner/mods. The 403 | |
| 156 | + | /// previously lived only on the moderation *page* GET while every mutation | |
| 157 | + | /// (ban/mute/pin/lock/mod-remove) gated on role alone, so a mod could keep | |
| 158 | + | /// moderating a suspended community. Every mutation must now 403. | |
| 159 | + | #[sqlx::test] | |
| 160 | + | async fn suspended_community_blocks_mod_mutations(_pool: sqlx::PgPool) { | |
| 161 | + | let mut h = TestHarness::new().await; | |
| 162 | + | ||
| 163 | + | let owner = h.login_as("suspmodowner").await; | |
| 164 | + | let community_id = h.create_community("Susp", "susp").await; | |
| 165 | + | h.add_membership(owner, community_id, "owner").await; | |
| 166 | + | let cat_id = h.create_category(community_id, "General", "general").await; | |
| 167 | + | let thread_id = h.create_thread_with_post(cat_id, owner, "Thread", "OP body").await; | |
| 168 | + | let post_id = mt_db::queries::list_posts_in_thread(&h.db, thread_id).await.unwrap()[0].id; | |
| 169 | + | ||
| 170 | + | // A victim member the mod would act on. | |
| 171 | + | let victim = h.login_as("suspvictim").await; | |
| 172 | + | h.add_membership(victim, community_id, "member").await; | |
| 173 | + | ||
| 174 | + | // Log in as the mod and seed a CSRF token from the still-live moderation page. | |
| 175 | + | let moduser = h.login_as("suspmoduser").await; | |
| 176 | + | h.add_membership(moduser, community_id, "moderator").await; | |
| 177 | + | let live = h.client.get("/p/susp/moderation").await; | |
| 178 | + | assert!(live.status.is_success(), "moderation page should render pre-suspension"); | |
| 179 | + | ||
| 180 | + | // Platform admin suspends the community. | |
| 181 | + | sqlx::query("UPDATE communities SET suspended_at = now() WHERE id = $1") | |
| 182 | + | .bind(community_id) | |
| 183 | + | .execute(&h.db) | |
| 184 | + | .await | |
| 185 | + | .unwrap(); | |
| 186 | + | ||
| 187 | + | // The moderation page GET now 403s... | |
| 188 | + | assert_eq!(h.client.get("/p/susp/moderation").await.status, StatusCode::FORBIDDEN); | |
| 189 | + | ||
| 190 | + | // ...and so must every mutation the mod attempts. | |
| 191 | + | let mutations: &[(&str, &str)] = &[ | |
| 192 | + | ("/p/susp/moderation/ban", "username=suspvictim&duration=permanent&reason=x"), | |
| 193 | + | ("/p/susp/moderation/mute", "username=suspvictim&duration=permanent"), | |
| 194 | + | (&format!("/p/susp/general/{thread_id}/pin"), ""), | |
| 195 | + | (&format!("/p/susp/general/{thread_id}/lock"), ""), | |
| 196 | + | (&format!("/p/susp/general/{thread_id}/posts/{post_id}/remove"), ""), | |
| 197 | + | ]; | |
| 198 | + | for (path, body) in mutations { | |
| 199 | + | let resp = h.client.post_form(path, body).await; | |
| 200 | + | assert_eq!( | |
| 201 | + | resp.status, | |
| 202 | + | StatusCode::FORBIDDEN, | |
| 203 | + | "suspended community must 403 the mutation at {path}, got {}", | |
| 204 | + | resp.status | |
| 205 | + | ); | |
| 206 | + | } | |
| 207 | + | ||
| 208 | + | // Nothing leaked through: the victim is neither banned nor muted. | |
| 209 | + | assert!(!mt_db::queries::is_user_banned(&h.db, community_id, victim).await.unwrap()); | |
| 210 | + | assert!(!mt_db::queries::is_user_muted(&h.db, community_id, victim).await.unwrap()); | |
| 211 | + | } | |
| 212 | + | ||
| 154 | 213 | #[sqlx::test] | |
| 155 | 214 | async fn mod_cannot_ban_other_mod(_pool: sqlx::PgPool) { | |
| 156 | 215 | let mut h = TestHarness::new().await; |