max / makenotwork

mt: block owner settings mutations on a suspended community The suspended-community 403 lived only on the settings GET page, so every owner-only write handler (rename, category create/edit/move, tag create/ delete, set_state) still executed. Fold the suspension check into require_owner (the shared gate for all owner-only settings routes) and add an is_platform_admin bypass on set_community_state so only the platform admin can still act on a suspended community. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-07-07 14:46 UTC

Commit: 1b1f65457374eb11ca54c501b79f8e406c22cd89

Parent: b97e123

3 files changed, +70 insertions, -7 deletions

M multithreaded/src/routes/helpers/authz.rs +10

			@@ -112,6 +112,13 @@ pub(crate) async fn check_write_access(
112	112		}
113	113
114	114		/// Helper: fetch community + verify owner role, returning 403 if not owner.
	115	+	///
	116	+	/// Also rejects a suspended community (403): a suspended-community owner must not
	117	+	/// be able to reach any settings route, GET or POST. All `require_owner` callers
	118	+	/// are owner-only settings handlers, so the suspension gate belongs here rather
	119	+	/// than duplicated per handler — see `docs/audit_review.md` (settings suspended
	120	+	/// authz gap). The platform admin acts on suspended communities via the `_admin`
	121	+	/// routes, not these.
115	122		#[tracing::instrument(skip_all)]
116	123		pub(crate) async fn require_owner(
117	124		state: &AppState,
			@@ -123,6 +130,9 @@ pub(crate) async fn require_owner(
123	130		if !is_owner(&role) {
124	131		return Err((StatusCode::FORBIDDEN, "Forbidden").into_response());
125	132		}
	133	+	if community.suspended_at.is_some() {
	134	+	return Err((StatusCode::FORBIDDEN, "This community has been suspended.").into_response());
	135	+	}
126	136		Ok(community)
127	137		}
128	138

M multithreaded/src/routes/settings.rs +10 -7

			@@ -16,9 +16,9 @@ use crate::AppState;
16	16		use mt_core::types::{CommunityState, ModAction, ModActor};
17	17
18	18		use super::{
19		-	audit, begin_tx, commit_tx, db_error, parse_uuid, require_mod_or_superadmin, require_owner, template_user,
20		-	validate_title, CreateCategoryForm, CreateTagForm, DeleteTagForm, EditCategoryFormData,
21		-	MoveCategoryForm, SetCommunityStateForm, UpdateCommunityForm,
	19	+	audit, begin_tx, commit_tx, db_error, is_platform_admin, parse_uuid, require_mod_or_superadmin,
	20	+	require_owner, template_user, validate_title, CreateCategoryForm, CreateTagForm, DeleteTagForm,
	21	+	EditCategoryFormData, MoveCategoryForm, SetCommunityStateForm, UpdateCommunityForm,
22	22		};
23	23
24	24		#[tracing::instrument(skip_all)]
			@@ -29,12 +29,9 @@ pub(super) async fn community_settings(
29	29		RequireUser(user): RequireUser,
30	30		) -> Result<impl IntoResponse, Response> {
31	31		let csrf_token = Some(csrf::get_or_create_token(&session).await);
	32	+	// `require_owner` already 403s a suspended community.
32	33		let community = require_owner(&state, &slug, &user).await?;
33	34
34		-	if community.suspended_at.is_some() {
35		-	return Err((StatusCode::FORBIDDEN, "This community has been suspended.").into_response());
36		-	}
37		-
38	35		let db_categories = mt_db::queries::list_categories_for_settings(&state.db, community.id)
39	36		.await
40	37		.map_err(db_error)?;
			@@ -357,6 +354,12 @@ pub(super) async fn set_community_state_handler(
357	354		) -> Result<Redirect, Response> {
358	355		let (community, _role) = require_mod_or_superadmin(&state, &slug, &user).await?;
359	356
	357	+	// A suspended community is frozen to its owner/mods: only the platform admin
	358	+	// (who manages suspensions via `_admin`) may still change its state.
	359	+	if community.suspended_at.is_some() && !is_platform_admin(&state, &user) {
	360	+	return Err((StatusCode::FORBIDDEN, "This community has been suspended.").into_response());
	361	+	}
	362	+
360	363		let new_state = CommunityState::from_db(form.state.trim()).ok_or_else(\|\| {
361	364		(
362	365		StatusCode::UNPROCESSABLE_ENTITY,

M multithreaded/tests/workflows/tags.rs +50

			@@ -17,6 +17,56 @@ async fn create_tag() {
17	17		assert_eq!(tags[0].slug, "bug");
18	18		}
19	19
	20	+	/// Regression (audit_review.md — settings suspended authz gap): a suspended
	21	+	/// community's owner must be blocked from mutating settings, not just from
	22	+	/// viewing the settings page. Prior to the fix `require_owner` let every write
	23	+	/// handler through because only the GET page checked `suspended_at`.
	24	+	#[tokio::test]
	25	+	async fn suspended_community_blocks_owner_settings_mutations() {
	26	+	let mut h = TestHarness::new().await;
	27	+	let owner_id = h.login_as("suspsettingsowner").await;
	28	+	let comm_id = h.create_community("Susp", "susp").await;
	29	+	h.add_membership(owner_id, comm_id, "owner").await;
	30	+	let cat_id = h.create_category(comm_id, "General", "general").await;
	31	+
	32	+	// Grab a session CSRF token from the still-live settings page.
	33	+	let live = h.client.get("/p/susp/settings").await;
	34	+	assert!(live.status.is_success(), "settings page should render pre-suspension");
	35	+
	36	+	// Platform-admin suspends the community (simulated directly).
	37	+	sqlx::query("UPDATE communities SET suspended_at = now() WHERE id = $1")
	38	+	.bind(comm_id)
	39	+	.execute(&h.db)
	40	+	.await
	41	+	.unwrap();
	42	+
	43	+	// The GET page 403s (existing behavior)...
	44	+	assert_eq!(h.client.get("/p/susp/settings").await.status.as_u16(), 403);
	45	+
	46	+	// ...and so must every owner-only mutation.
	47	+	let mutations: &[(&str, String)] = &[
	48	+	("/p/susp/settings", "name=Renamed&description=x&auto_hide_threshold=".to_string()),
	49	+	("/p/susp/settings/categories/new", "name=Cat&slug=cat&description=".to_string()),
	50	+	(&format!("/p/susp/settings/categories/{cat_id}/edit"), "name=Renamed&description=".to_string()),
	51	+	(&format!("/p/susp/settings/categories/{cat_id}/move"), "direction=up".to_string()),
	52	+	("/p/susp/settings/tags/new", "name=Bug&slug=bug".to_string()),
	53	+	("/p/susp/settings/state", "state=frozen".to_string()),
	54	+	];
	55	+	for (path, body) in mutations {
	56	+	let resp = h.client.post_form(path, body).await;
	57	+	assert_eq!(
	58	+	resp.status.as_u16(),
	59	+	403,
	60	+	"suspended community must 403 the mutation at {path}, got {}",
	61	+	resp.status
	62	+	);
	63	+	}
	64	+
	65	+	// Nothing leaked through: no tag was created, name unchanged.
	66	+	let tags = mt_db::queries::list_tags_for_community(&h.db, comm_id).await.unwrap();
	67	+	assert!(tags.is_empty(), "no tag should have been created on a suspended community");
	68	+	}
	69	+
20	70		#[tokio::test]
21	71		async fn apply_tag_to_thread() {
22	72		let mut h = TestHarness::new().await;