max / makenotwork
14 files changed,
+304 insertions,
-137 deletions
| @@ -0,0 +1,15 @@ | |||
| 1 | + | # Seal the C1 resource-in-community invariant (ultra-fuzz CHRONIC). | |
| 2 | + | # | |
| 3 | + | # These raw by-id loaders return a community resource without proving it belongs | |
| 4 | + | # to the URL slug's community. Handler code must instead go through | |
| 5 | + | # `crate::routes::scope::CommunityScope::resolve`, which performs the equality | |
| 6 | + | # check. The only sanctioned callers are the `ScopedResource::load` impls in | |
| 7 | + | # `src/routes/scope.rs`, which carry a local `#[allow(clippy::disallowed_methods)]`. | |
| 8 | + | # | |
| 9 | + | # Under the `-D warnings` CI gate, calling either from anywhere else is a build | |
| 10 | + | # failure — so a handler that loads a community resource without the check no | |
| 11 | + | # longer compiles. That is the structural resolution of C1. | |
| 12 | + | disallowed-methods = [ | |
| 13 | + | { path = "mt_db::queries::get_thread_with_breadcrumb", reason = "load via routes::scope::CommunityScope so the resource is proven to belong to the URL slug's community (C1)" }, | |
| 14 | + | { path = "mt_db::queries::get_post_for_edit", reason = "load via routes::scope::CommunityScope so the resource is proven to belong to the URL slug's community (C1)" }, | |
| 15 | + | ] |
| @@ -486,8 +486,13 @@ pub async fn callback( | |||
| 486 | 486 | tracing::warn!(error = %e, "failed to remove PKCE verifier from session"); | |
| 487 | 487 | } | |
| 488 | 488 | ||
| 489 | - | // Verify state nonce | |
| 490 | - | if stored_state.as_deref() != Some(¶ms.state) { | |
| 489 | + | // Verify state nonce in constant time — it's a CSRF token, so compare it on | |
| 490 | + | // the same timing-safe path as every other secret (no early-exit on length | |
| 491 | + | // or first differing byte). | |
| 492 | + | let state_ok = stored_state | |
| 493 | + | .as_deref() | |
| 494 | + | .is_some_and(|s| crate::csrf::constant_time_compare(s, ¶ms.state)); | |
| 495 | + | if !state_ok { | |
| 491 | 496 | tracing::warn!(stored = ?stored_state, received = %params.state, "state mismatch"); | |
| 492 | 497 | return Redirect::to("/?error=state_mismatch"); | |
| 493 | 498 | } |
| @@ -48,10 +48,11 @@ impl S3Config { | |||
| 48 | 48 | ||
| 49 | 49 | impl Config { | |
| 50 | 50 | pub fn from_env() -> Self { | |
| 51 | + | let mnw_base_url = | |
| 52 | + | std::env::var("MNW_BASE_URL").unwrap_or_else(|_| "http://127.0.0.1:3000".to_string()); | |
| 53 | + | assert_secure_base_url(&mnw_base_url); | |
| 51 | 54 | Self { | |
| 52 | - | mnw_base_url: std::env::var("MNW_BASE_URL") | |
| 53 | - | .unwrap_or_else(|_| "http://127.0.0.1:3000".to_string()) | |
| 54 | - | .into(), | |
| 55 | + | mnw_base_url: mnw_base_url.into(), | |
| 55 | 56 | oauth_client_id: std::env::var("OAUTH_CLIENT_ID") | |
| 56 | 57 | .expect("OAUTH_CLIENT_ID must be set"), | |
| 57 | 58 | oauth_redirect_uri: std::env::var("OAUTH_REDIRECT_URI") | |
| @@ -69,6 +70,21 @@ impl Config { | |||
| 69 | 70 | } | |
| 70 | 71 | } | |
| 71 | 72 | ||
| 73 | + | /// Refuse to boot a real deployment with a non-`https` base URL. | |
| 74 | + | /// | |
| 75 | + | /// A non-loopback `MNW_BASE_URL` served over plain `http` ships OAuth redirects, | |
| 76 | + | /// session cookies, and absolute links over cleartext. Loopback hosts | |
| 77 | + | /// (`127.0.0.1`, `localhost`, `[::1]`) are exempt so local HTTP development still | |
| 78 | + | /// works; anything else must be `https`. | |
| 79 | + | fn assert_secure_base_url(url: &str) { | |
| 80 | + | let is_loopback = | |
| 81 | + | url.contains("127.0.0.1") || url.contains("localhost") || url.contains("[::1]"); | |
| 82 | + | assert!( | |
| 83 | + | is_loopback || url.starts_with("https://"), | |
| 84 | + | "MNW_BASE_URL must be https for a non-loopback deployment (got `{url}`)" | |
| 85 | + | ); | |
| 86 | + | } | |
| 87 | + | ||
| 72 | 88 | /// Parse `TRUSTED_PROXIES` (comma-separated IPs). Unset → loopback only; an | |
| 73 | 89 | /// explicit empty value → trust no proxy (every request keys on its peer). | |
| 74 | 90 | /// Unparseable entries are skipped with a warning rather than failing boot. |
| @@ -14,9 +14,10 @@ use crate::AppState; | |||
| 14 | 14 | use mt_core::types::{ModAction, ModActor}; | |
| 15 | 15 | ||
| 16 | 16 | use super::{ | |
| 17 | - | audit, begin_tx, check_community_access, commit_tx, field_error, get_community, parse_uuid, | |
| 18 | - | require_mod_or_owner, | |
| 17 | + | audit, begin_tx, check_community_access, commit_tx, field_error, parse_uuid, | |
| 18 | + | require_mod_or_owner, CommunityScope, | |
| 19 | 19 | }; | |
| 20 | + | use mt_db::queries::PostForEdit; | |
| 20 | 21 | ||
| 21 | 22 | #[derive(Deserialize)] | |
| 22 | 23 | pub(super) struct FlagForm { | |
| @@ -35,36 +36,25 @@ pub(super) async fn flag_post_handler( | |||
| 35 | 36 | let user = session_user | |
| 36 | 37 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 37 | 38 | ||
| 38 | - | let post_id = parse_uuid(&post_id_str)?; | |
| 39 | - | ||
| 40 | 39 | // Validate reason | |
| 41 | 40 | if !matches!(form.reason.as_str(), "spam" | "rule_breaking" | "off_topic") { | |
| 42 | 41 | return Err(field_error("reason", "Invalid flag reason.")); | |
| 43 | 42 | } | |
| 44 | 43 | ||
| 45 | - | // Fetch post to check ownership and existence | |
| 46 | - | let post_data = mt_db::queries::get_post_for_edit(&state.db, post_id) | |
| 47 | - | .await | |
| 48 | - | .map_err(|e| { | |
| 49 | - | tracing::error!(error = ?e, "db error fetching post for flag"); | |
| 50 | - | (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error").into_response() | |
| 51 | - | })? | |
| 52 | - | .ok_or_else(|| (StatusCode::NOT_FOUND, "Not found").into_response())?; | |
| 44 | + | // Resolve the post within the slug's community — CommunityScope proves the | |
| 45 | + | // post belongs here, so a user banned in the post's community can't route the | |
| 46 | + | // flag through a different slug to evade the ban check or apply the wrong | |
| 47 | + | // community's auto-hide threshold (replaces the old hand-copied guard). | |
| 48 | + | let scope = CommunityScope::<PostForEdit>::resolve(&state.db, &slug, &post_id_str).await?; | |
| 49 | + | let post_id = scope.resource.id; | |
| 53 | 50 | ||
| 54 | 51 | // Cannot flag own post | |
| 55 | - | if user.user_id == post_data.author_id { | |
| 52 | + | if user.user_id == scope.resource.author_id { | |
| 56 | 53 | return Err((StatusCode::FORBIDDEN, "You cannot flag your own post.").into_response()); | |
| 57 | 54 | } | |
| 58 | 55 | ||
| 59 | - | // Check community access. Authorize against the post's *own* community — | |
| 60 | - | // not just the URL slug — so a user banned in the post's community can't | |
| 61 | - | // route the flag through a different slug to evade the ban check and apply | |
| 62 | - | // the wrong community's auto-hide threshold. | |
| 63 | - | let community = get_community(&state.db, &slug).await?; | |
| 64 | - | if post_data.community_id != community.id { | |
| 65 | - | return Err((StatusCode::NOT_FOUND, "Not found").into_response()); | |
| 66 | - | } | |
| 67 | - | check_community_access(&state.db, &community, Some(user.user_id)).await?; | |
| 56 | + | check_community_access(&state.db, &scope.community, Some(user.user_id)).await?; | |
| 57 | + | let CommunityScope { community, resource: post_data } = scope; | |
| 68 | 58 | ||
| 69 | 59 | let detail = form.detail.as_deref().filter(|d| !d.trim().is_empty()); | |
| 70 | 60 |
| @@ -12,10 +12,11 @@ use crate::auth::MaybeUser; | |||
| 12 | 12 | use crate::AppState; | |
| 13 | 13 | ||
| 14 | 14 | use super::super::{ | |
| 15 | - | check_community_access, check_user_post_rate, check_write_access, check_write_state, get_community, WriteScope, | |
| 16 | - | parse_uuid, validate_body, FootnoteForm, | |
| 15 | + | check_community_access, check_user_post_rate, check_write_state, CommunityScope, WriteScope, | |
| 16 | + | validate_body, FootnoteForm, | |
| 17 | 17 | }; | |
| 18 | 18 | use super::posts::{resolve_and_render_mentions, MAX_FOOTNOTES_PER_POST}; | |
| 19 | + | use mt_db::queries::PostForEdit; | |
| 19 | 20 | ||
| 20 | 21 | /// Why a footnote add was rejected. Pure predicate result; the handler | |
| 21 | 22 | /// translates each variant to an HTTP response. | |
| @@ -86,15 +87,8 @@ pub(in crate::routes) async fn add_footnote_handler( | |||
| 86 | 87 | let user = session_user | |
| 87 | 88 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 88 | 89 | ||
| 89 | - | let post_id = parse_uuid(&post_id_str)?; | |
| 90 | - | ||
| 91 | - | let post_data = mt_db::queries::get_post_for_edit(&state.db, post_id) | |
| 92 | - | .await | |
| 93 | - | .map_err(|e| { | |
| 94 | - | tracing::error!(error = ?e, "db error fetching post for footnote"); | |
| 95 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 96 | - | })? | |
| 97 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 90 | + | let scope = CommunityScope::<PostForEdit>::resolve(&state.db, &slug, &post_id_str).await?; | |
| 91 | + | let post_id = scope.resource.id; | |
| 98 | 92 | ||
| 99 | 93 | let removed: bool = sqlx::query_scalar("SELECT removed_at IS NOT NULL FROM posts WHERE id = $1") | |
| 100 | 94 | .bind(post_id) | |
| @@ -112,7 +106,7 @@ pub(in crate::routes) async fn add_footnote_handler( | |||
| 112 | 106 | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 113 | 107 | })?; | |
| 114 | 108 | ||
| 115 | - | check_footnote_permission(user.user_id, post_data.author_id, removed, footnote_count) | |
| 109 | + | check_footnote_permission(user.user_id, scope.resource.author_id, removed, footnote_count) | |
| 116 | 110 | .map_err(|denial| match denial { | |
| 117 | 111 | FootnoteDenial::NotAuthor | FootnoteDenial::PostRemoved => { | |
| 118 | 112 | StatusCode::FORBIDDEN.into_response() | |
| @@ -124,16 +118,11 @@ pub(in crate::routes) async fn add_footnote_handler( | |||
| 124 | 118 | .into_response(), | |
| 125 | 119 | })?; | |
| 126 | 120 | ||
| 127 | - | // Check write access (suspension + ban + mute) | |
| 128 | - | let community = get_community(&state.db, &slug).await?; | |
| 129 | - | // Authorize against the post's *own* community, not just the URL slug, so a | |
| 130 | - | // user banned/muted in the post's community can't route the footnote through | |
| 131 | - | // a different slug to evade the check (matches the flag/endorse/remove paths). | |
| 132 | - | if post_data.community_id != community.id { | |
| 133 | - | return Err((StatusCode::NOT_FOUND, "Not found").into_response()); | |
| 134 | - | } | |
| 135 | - | ||
| 136 | - | check_write_access(&state.db, community.id, user.user_id, community.suspended_at.is_some()).await?; | |
| 121 | + | // Write access (suspension + ban + mute) against the scope-verified community; | |
| 122 | + | // CommunityScope already proved the post belongs to this slug's community, so | |
| 123 | + | // the old hand-copied `post_data.community_id != community.id` guard is gone. | |
| 124 | + | scope.require_write_access(&state.db, user.user_id).await?; | |
| 125 | + | let community = scope.community; | |
| 137 | 126 | check_write_state(&state, &community, &user, WriteScope::ContinueExisting).await?; | |
| 138 | 127 | mt_db::mutations::ensure_membership(&state.db, user.user_id, community.id) | |
| 139 | 128 | .await | |
| @@ -178,15 +167,8 @@ pub(in crate::routes) async fn toggle_endorsement_handler( | |||
| 178 | 167 | let user = session_user | |
| 179 | 168 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 180 | 169 | ||
| 181 | - | let post_id = parse_uuid(&post_id_str)?; | |
| 182 | - | ||
| 183 | - | let post_data = mt_db::queries::get_post_for_edit(&state.db, post_id) | |
| 184 | - | .await | |
| 185 | - | .map_err(|e| { | |
| 186 | - | tracing::error!(error = ?e, "db error fetching post for endorsement"); | |
| 187 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 188 | - | })? | |
| 189 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 170 | + | let scope = CommunityScope::<PostForEdit>::resolve(&state.db, &slug, &post_id_str).await?; | |
| 171 | + | let post_id = scope.resource.id; | |
| 190 | 172 | ||
| 191 | 173 | let removed: bool = sqlx::query_scalar("SELECT removed_at IS NOT NULL FROM posts WHERE id = $1") | |
| 192 | 174 | .bind(post_id) | |
| @@ -197,14 +179,10 @@ pub(in crate::routes) async fn toggle_endorsement_handler( | |||
| 197 | 179 | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 198 | 180 | })?; | |
| 199 | 181 | ||
| 200 | - | // Check community access (suspension + ban) — no mute check since endorsing is not content. | |
| 201 | - | // Authorize against the post's *own* community, not just the URL slug, so a | |
| 202 | - | // user banned in the post's community can't route the endorsement through a | |
| 203 | - | // different slug to evade the ban check (matches the flag/remove paths). | |
| 204 | - | let community = get_community(&state.db, &slug).await?; | |
| 205 | - | if post_data.community_id != community.id { | |
| 206 | - | return Err((StatusCode::NOT_FOUND, "Not found").into_response()); | |
| 207 | - | } | |
| 182 | + | // Community access (suspension + ban) against the scope-verified community — | |
| 183 | + | // no mute check since endorsing is not content. CommunityScope already proved | |
| 184 | + | // the post belongs to this slug's community. | |
| 185 | + | let community = scope.community; | |
| 208 | 186 | check_community_access(&state.db, &community, Some(user.user_id)).await?; | |
| 209 | 187 | // Endorsement is a write action, so it's blocked by Frozen/Archived state. | |
| 210 | 188 | check_write_state(&state, &community, &user, WriteScope::ContinueExisting).await?; | |
| @@ -216,7 +194,7 @@ pub(in crate::routes) async fn toggle_endorsement_handler( | |||
| 216 | 194 | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 217 | 195 | })?; | |
| 218 | 196 | ||
| 219 | - | check_endorsement_permission(user.user_id, post_data.author_id, removed, suspended) | |
| 197 | + | check_endorsement_permission(user.user_id, scope.resource.author_id, removed, suspended) | |
| 220 | 198 | .map_err(|denial| match denial { | |
| 221 | 199 | EndorsementDenial::CannotEndorseOwn | EndorsementDenial::PostRemoved => { | |
| 222 | 200 | StatusCode::FORBIDDEN.into_response() |
| @@ -18,12 +18,13 @@ use mt_core::types::ModAction; | |||
| 18 | 18 | ||
| 19 | 19 | use super::super::{ | |
| 20 | 20 | audit, begin_tx, check_user_post_rate, check_write_access, check_write_state, commit_tx, | |
| 21 | - | get_community, get_thread, is_mod_or_owner, parse_uuid, reject_embeds_for_free_user, | |
| 21 | + | get_community, is_mod_or_owner, parse_uuid, reject_embeds_for_free_user, | |
| 22 | 22 | render_markdown, render_markdown_plus, render_markdown_with_mentions, template_user, | |
| 23 | - | validate_body, validate_title, get_role, WriteScope, | |
| 23 | + | validate_body, validate_title, CommunityScope, WriteScope, | |
| 24 | 24 | CreateReplyForm, CreateThreadForm, | |
| 25 | 25 | }; | |
| 26 | 26 | use mt_core::types::ModActor; | |
| 27 | + | use mt_db::queries::ThreadWithBreadcrumb; | |
| 27 | 28 | ||
| 28 | 29 | // ============================================================================ | |
| 29 | 30 | // Quote verification | |
| @@ -333,10 +334,11 @@ pub(in crate::routes) async fn create_reply_handler( | |||
| 333 | 334 | let user = session_user | |
| 334 | 335 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 335 | 336 | ||
| 336 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 337 | - | let community = get_community(&state.db, &slug).await?; | |
| 337 | + | let scope = | |
| 338 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 339 | + | scope.require_write_access(&state.db, user.user_id).await?; | |
| 340 | + | let CommunityScope { community, resource: thread_data } = scope; | |
| 338 | 341 | ||
| 339 | - | check_write_access(&state.db, community.id, user.user_id, community.suspended_at.is_some()).await?; | |
| 340 | 342 | check_write_state(&state, &community, &user, WriteScope::ContinueExisting).await?; | |
| 341 | 343 | mt_db::mutations::ensure_membership(&state.db, user.user_id, community.id) | |
| 342 | 344 | .await | |
| @@ -402,15 +404,14 @@ pub(in crate::routes) async fn edit_thread_form( | |||
| 402 | 404 | let user = session_user | |
| 403 | 405 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 404 | 406 | ||
| 405 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 406 | - | let community = get_community(&state.db, &slug).await?; | |
| 407 | - | ||
| 408 | - | check_write_access(&state.db, community.id, user.user_id, community.suspended_at.is_some()).await?; | |
| 409 | - | ||
| 410 | - | let role = get_role(&state.db, user.user_id, thread_data.community_id).await?; | |
| 407 | + | let scope = | |
| 408 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 409 | + | scope.require_write_access(&state.db, user.user_id).await?; | |
| 410 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 411 | 411 | if !is_mod_or_owner(&role) { | |
| 412 | 412 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 413 | 413 | } | |
| 414 | + | let thread_data = scope.resource; | |
| 414 | 415 | ||
| 415 | 416 | Ok(crate::templates::EditThreadTemplate { | |
| 416 | 417 | csrf_token, | |
| @@ -435,12 +436,10 @@ pub(in crate::routes) async fn edit_thread_handler( | |||
| 435 | 436 | let user = session_user | |
| 436 | 437 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 437 | 438 | ||
| 438 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 439 | - | let community = get_community(&state.db, &slug).await?; | |
| 440 | - | ||
| 441 | - | check_write_access(&state.db, community.id, user.user_id, community.suspended_at.is_some()).await?; | |
| 442 | - | ||
| 443 | - | let role = get_role(&state.db, user.user_id, thread_data.community_id).await?; | |
| 439 | + | let scope = | |
| 440 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 441 | + | scope.require_write_access(&state.db, user.user_id).await?; | |
| 442 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 444 | 443 | if !is_mod_or_owner(&role) { | |
| 445 | 444 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 446 | 445 | } | |
| @@ -469,15 +468,14 @@ pub(in crate::routes) async fn delete_thread_handler( | |||
| 469 | 468 | let user = session_user | |
| 470 | 469 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 471 | 470 | ||
| 472 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 473 | - | let community = get_community(&state.db, &slug).await?; | |
| 474 | - | ||
| 475 | - | check_write_access(&state.db, community.id, user.user_id, community.suspended_at.is_some()).await?; | |
| 476 | - | ||
| 477 | - | let role = get_role(&state.db, user.user_id, thread_data.community_id).await?; | |
| 471 | + | let scope = | |
| 472 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 473 | + | scope.require_write_access(&state.db, user.user_id).await?; | |
| 474 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 478 | 475 | if !is_mod_or_owner(&role) { | |
| 479 | 476 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 480 | 477 | } | |
| 478 | + | let thread_data = scope.resource; | |
| 481 | 479 | ||
| 482 | 480 | let thread_id = parse_uuid(&thread_id_str)?; | |
| 483 | 481 | let mut tx = begin_tx(&state.db).await?; |
| @@ -15,9 +15,9 @@ use crate::AppState; | |||
| 15 | 15 | use std::collections::HashMap; | |
| 16 | 16 | ||
| 17 | 17 | use super::super::{ | |
| 18 | - | check_community_access, get_community, get_role, get_thread, is_mod_or_owner, | |
| 19 | - | parse_uuid, template_user, PageQuery, | |
| 18 | + | check_community_access, is_mod_or_owner, parse_uuid, template_user, CommunityScope, PageQuery, | |
| 20 | 19 | }; | |
| 20 | + | use mt_db::queries::ThreadWithBreadcrumb; | |
| 21 | 21 | ||
| 22 | 22 | #[tracing::instrument(skip_all)] | |
| 23 | 23 | pub(in crate::routes) async fn thread( | |
| @@ -29,10 +29,17 @@ pub(in crate::routes) async fn thread( | |||
| 29 | 29 | ) -> Result<impl IntoResponse, Response> { | |
| 30 | 30 | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 31 | 31 | ||
| 32 | - | let thread_data = get_thread(&state.db, &thread_id).await?; | |
| 33 | - | let community = get_community(&state.db, &slug).await?; | |
| 32 | + | let scope = CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id).await?; | |
| 33 | + | check_community_access(&state.db, &scope.community, session_user.as_ref().map(|u| u.user_id)).await?; | |
| 34 | 34 | ||
| 35 | - | check_community_access(&state.db, &community, session_user.as_ref().map(|u| u.user_id)).await?; | |
| 35 | + | // Role lookup must precede moving the resource out of the scope (it borrows | |
| 36 | + | // the verified community). | |
| 37 | + | let role = if let Some(ref user) = session_user { | |
| 38 | + | scope.role(&state.db, user.user_id).await? | |
| 39 | + | } else { | |
| 40 | + | None | |
| 41 | + | }; | |
| 42 | + | let thread_data = scope.resource; | |
| 36 | 43 | ||
| 37 | 44 | let per_page: i64 = 50; | |
| 38 | 45 | ||
| @@ -56,13 +63,6 @@ pub(in crate::routes) async fn thread( | |||
| 56 | 63 | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 57 | 64 | })?; | |
| 58 | 65 | ||
| 59 | - | // Look up user's role in this community (if logged in) | |
| 60 | - | let role = if let Some(ref user) = session_user { | |
| 61 | - | get_role(&state.db, user.user_id, thread_data.community_id).await? | |
| 62 | - | } else { | |
| 63 | - | None | |
| 64 | - | }; | |
| 65 | - | ||
| 66 | 66 | let mod_status = is_mod_or_owner(&role); | |
| 67 | 67 | ||
| 68 | 68 | // Check tracking status and update read position |
| @@ -101,22 +101,6 @@ pub(crate) async fn get_community( | |||
| 101 | 101 | .ok_or_else(crate::error_page::not_found) | |
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | - | /// Fetch thread with breadcrumb, returning 404/500 on failure. | |
| 105 | - | #[tracing::instrument(skip_all)] | |
| 106 | - | pub(crate) async fn get_thread( | |
| 107 | - | db: &sqlx::PgPool, | |
| 108 | - | thread_id_str: &str, | |
| 109 | - | ) -> Result<mt_db::queries::ThreadWithBreadcrumb, Response> { | |
| 110 | - | let thread_id = parse_uuid(thread_id_str)?; | |
| 111 | - | mt_db::queries::get_thread_with_breadcrumb(db, thread_id) | |
| 112 | - | .await | |
| 113 | - | .map_err(|e| { | |
| 114 | - | tracing::error!(error = ?e, "db error fetching thread"); | |
| 115 | - | crate::error_page::internal_error() | |
| 116 | - | })? | |
| 117 | - | .ok_or_else(crate::error_page::not_found) | |
| 118 | - | } | |
| 119 | - | ||
| 120 | 104 | /// Parse a UUID from a string, returning 404 on failure. | |
| 121 | 105 | #[allow(clippy::result_large_err)] | |
| 122 | 106 | pub(crate) fn parse_uuid(id_str: &str) -> Result<Uuid, Response> { |
| @@ -331,7 +331,11 @@ async fn create_post( | |||
| 331 | 331 | .await | |
| 332 | 332 | .map_err(db_error)?; | |
| 333 | 333 | ||
| 334 | - | // Look up thread's community and ensure membership | |
| 334 | + | // Look up thread's community and ensure membership. This is the internal | |
| 335 | + | // server-to-server API (no URL slug to scope against), so the CommunityScope | |
| 336 | + | // resolver doesn't apply — the thread id is supplied directly by the trusted | |
| 337 | + | // internal caller, not derived from a `/p/{slug}/…` path. | |
| 338 | + | #[allow(clippy::disallowed_methods)] // internal API: no slug to verify against | |
| 335 | 339 | let thread_info = mt_db::queries::get_thread_with_breadcrumb(&state.db, thread_id) | |
| 336 | 340 | .await | |
| 337 | 341 | .map_err(db_error)? |
| @@ -7,6 +7,7 @@ mod forum; | |||
| 7 | 7 | mod helpers; | |
| 8 | 8 | pub mod internal; | |
| 9 | 9 | mod moderation; | |
| 10 | + | mod scope; | |
| 10 | 11 | mod search; | |
| 11 | 12 | mod settings; | |
| 12 | 13 | mod tracking; | |
| @@ -14,6 +15,7 @@ mod uploads; | |||
| 14 | 15 | ||
| 15 | 16 | // Re-export helpers so submodules can `use super::*` as before. | |
| 16 | 17 | pub(crate) use helpers::*; | |
| 18 | + | pub(crate) use scope::CommunityScope; | |
| 17 | 19 | ||
| 18 | 20 | use axum::{ | |
| 19 | 21 | http::StatusCode, |
| @@ -16,11 +16,12 @@ use crate::AppState; | |||
| 16 | 16 | use mt_core::types::{BanType, ModAction}; | |
| 17 | 17 | ||
| 18 | 18 | use super::{ | |
| 19 | - | audit, begin_tx, commit_tx, field_error, get_role, get_thread, get_user_by_username, | |
| 20 | - | is_mod_or_owner, is_owner, parse_duration, parse_uuid, require_mod_or_owner, template_user, | |
| 19 | + | audit, begin_tx, commit_tx, field_error, get_role, get_user_by_username, is_mod_or_owner, | |
| 20 | + | is_owner, parse_duration, parse_uuid, require_mod_or_owner, template_user, CommunityScope, | |
| 21 | 21 | BanForm, PageQuery, UnbanForm, | |
| 22 | 22 | }; | |
| 23 | 23 | use mt_core::types::ModActor; | |
| 24 | + | use mt_db::queries::{PostForEdit, ThreadWithBreadcrumb}; | |
| 24 | 25 | ||
| 25 | 26 | #[tracing::instrument(skip_all)] | |
| 26 | 27 | pub(super) async fn pin_thread_handler( | |
| @@ -31,12 +32,14 @@ pub(super) async fn pin_thread_handler( | |||
| 31 | 32 | let user = session_user | |
| 32 | 33 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 33 | 34 | ||
| 34 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 35 | - | let role = get_role(&state.db, user.user_id, thread_data.community_id).await?; | |
| 35 | + | let scope = | |
| 36 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 37 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 36 | 38 | ||
| 37 | 39 | if !is_mod_or_owner(&role) { | |
| 38 | 40 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 39 | 41 | } | |
| 42 | + | let thread_data = scope.resource; | |
| 40 | 43 | ||
| 41 | 44 | let new_pinned = !thread_data.pinned; | |
| 42 | 45 | let action = if new_pinned { ModAction::PinThread } else { ModAction::UnpinThread }; | |
| @@ -69,12 +72,14 @@ pub(super) async fn lock_thread_handler( | |||
| 69 | 72 | let user = session_user | |
| 70 | 73 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 71 | 74 | ||
| 72 | - | let thread_data = get_thread(&state.db, &thread_id_str).await?; | |
| 73 | - | let role = get_role(&state.db, user.user_id, thread_data.community_id).await?; | |
| 75 | + | let scope = | |
| 76 | + | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 77 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 74 | 78 | ||
| 75 | 79 | if !is_mod_or_owner(&role) { | |
| 76 | 80 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 77 | 81 | } | |
| 82 | + | let thread_data = scope.resource; | |
| 78 | 83 | ||
| 79 | 84 | let new_locked = !thread_data.locked; | |
| 80 | 85 | let action = if new_locked { ModAction::LockThread } else { ModAction::UnlockThread }; | |
| @@ -111,21 +116,14 @@ pub(super) async fn mod_remove_post_handler( | |||
| 111 | 116 | let user = session_user | |
| 112 | 117 | .ok_or_else(|| Redirect::to("/auth/login").into_response())?; | |
| 113 | 118 | ||
| 114 | - | let post_id = parse_uuid(&post_id_str)?; | |
| 115 | - | ||
| 116 | - | let post_data = mt_db::queries::get_post_for_edit(&state.db, post_id) | |
| 117 | - | .await | |
| 118 | - | .map_err(|e| { | |
| 119 | - | tracing::error!(error = ?e, "db error fetching post for removal"); | |
| 120 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 121 | - | })? | |
| 122 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 123 | - | ||
| 124 | - | let role = get_role(&state.db, user.user_id, post_data.community_id).await?; | |
| 119 | + | let scope = CommunityScope::<PostForEdit>::resolve(&state.db, &slug, &post_id_str).await?; | |
| 120 | + | let role = scope.role(&state.db, user.user_id).await?; | |
| 125 | 121 | ||
| 126 | 122 | if !is_mod_or_owner(&role) { | |
| 127 | 123 | return Err(StatusCode::FORBIDDEN.into_response()); | |
| 128 | 124 | } | |
| 125 | + | let post_data = scope.resource; | |
| 126 | + | let post_id = post_data.id; | |
| 129 | 127 | ||
| 130 | 128 | // Resolve the thread id up front so the cascade-delete log can be written on | |
| 131 | 129 | // the same transaction as the removal. |
| @@ -0,0 +1,132 @@ | |||
| 1 | + | //! `CommunityScope` — the single, sealed way to load a resource together with | |
| 2 | + | //! the community its URL slug names, proving the two belong together. | |
| 3 | + | //! | |
| 4 | + | //! ## The chronic this closes (ultra-fuzz C1, 3 consecutive runs) | |
| 5 | + | //! | |
| 6 | + | //! Handlers reached via `/p/{slug}/…/{resource_id}` must serve a resource whose | |
| 7 | + | //! `community_id` equals the community resolved from `{slug}`, and *all* access | |
| 8 | + | //! predicates (ban/mute/suspension, role) must evaluate against that one | |
| 9 | + | //! community — not a mix of the slug's community and the resource's real one. | |
| 10 | + | //! | |
| 11 | + | //! For three runs the guard was a per-call-site convention: a hand-copied | |
| 12 | + | //! `if resource.community_id != community.id { 404 }` line. It drifted every | |
| 13 | + | //! time a new handler was written without it (run #5: `edit_thread`/ | |
| 14 | + | //! `delete_thread` split their predicates across two different communities). | |
| 15 | + | //! Copying the line onto two more handlers is a fourth instance of the failed | |
| 16 | + | //! fix class, not a resolution. | |
| 17 | + | //! | |
| 18 | + | //! ## The structural fix | |
| 19 | + | //! | |
| 20 | + | //! [`CommunityScope::resolve`] is the *only* way to obtain a community-scoped | |
| 21 | + | //! resource. It loads both the slug's community and the resource by id, returns | |
| 22 | + | //! 404 unless their `community_id`s match, and yields one value carrying the | |
| 23 | + | //! verified community alongside the resource. Access checks | |
| 24 | + | //! ([`CommunityScope::require_write_access`], [`CommunityScope::role`]) read the | |
| 25 | + | //! community *from that value*, so they cannot be handed a divergent community | |
| 26 | + | //! id. The raw by-id loaders (`get_thread_with_breadcrumb`, `get_post_for_edit`) | |
| 27 | + | //! are barred from handler code by a `disallowed-methods` clippy lint (see | |
| 28 | + | //! `clippy.toml`) that is allowed *only* inside this module — so a handler that | |
| 29 | + | //! loads a community resource without the equality check no longer compiles | |
| 30 | + | //! under the `-D warnings` gate. | |
| 31 | + | ||
| 32 | + | use axum::response::Response; | |
| 33 | + | use uuid::Uuid; | |
| 34 | + | ||
| 35 | + | use mt_core::types::CommunityRole; | |
| 36 | + | use mt_db::queries::{CommunityRow, PostForEdit, ThreadWithBreadcrumb}; | |
| 37 | + | ||
| 38 | + | use super::{check_write_access, get_community, get_role, parse_uuid}; | |
| 39 | + | ||
| 40 | + | /// A resource that lives inside a community and can be loaded by id. | |
| 41 | + | /// | |
| 42 | + | /// Implemented only for the resource types that hang off a `/p/{slug}/…` path. | |
| 43 | + | /// The `load` impls are the sole sanctioned callers of the raw mt-db by-id | |
| 44 | + | /// loaders; everything else must go through [`CommunityScope`]. | |
| 45 | + | pub(crate) trait ScopedResource: Sized + Send { | |
| 46 | + | fn load( | |
| 47 | + | db: &sqlx::PgPool, | |
| 48 | + | id: Uuid, | |
| 49 | + | ) -> impl std::future::Future<Output = Result<Option<Self>, sqlx::Error>> + Send; | |
| 50 | + | ||
| 51 | + | fn community_id(&self) -> Uuid; | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | impl ScopedResource for ThreadWithBreadcrumb { | |
| 55 | + | async fn load(db: &sqlx::PgPool, id: Uuid) -> Result<Option<Self>, sqlx::Error> { | |
| 56 | + | #[allow(clippy::disallowed_methods)] // the one sanctioned call site | |
| 57 | + | mt_db::queries::get_thread_with_breadcrumb(db, id).await | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | fn community_id(&self) -> Uuid { | |
| 61 | + | self.community_id | |
| 62 | + | } | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | impl ScopedResource for PostForEdit { | |
| 66 | + | async fn load(db: &sqlx::PgPool, id: Uuid) -> Result<Option<Self>, sqlx::Error> { | |
| 67 | + | #[allow(clippy::disallowed_methods)] // the one sanctioned call site | |
| 68 | + | mt_db::queries::get_post_for_edit(db, id).await | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | fn community_id(&self) -> Uuid { | |
| 72 | + | self.community_id | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | /// A resource verified to belong to the community its URL slug names. | |
| 77 | + | /// | |
| 78 | + | /// Construct only via [`CommunityScope::resolve`]. The fields are public to the | |
| 79 | + | /// crate so handlers can read `resource` and `community`, but there is no way to | |
| 80 | + | /// build the struct with a mismatched pair. | |
| 81 | + | pub(crate) struct CommunityScope<T> { | |
| 82 | + | pub(crate) community: CommunityRow, | |
| 83 | + | pub(crate) resource: T, | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | impl<T: ScopedResource> CommunityScope<T> { | |
| 87 | + | /// Load the slug's community and the resource by id, returning 404 unless the | |
| 88 | + | /// resource belongs to that community. This single check is the C1 invariant. | |
| 89 | + | #[allow(clippy::result_large_err)] | |
| 90 | + | pub(crate) async fn resolve( | |
| 91 | + | db: &sqlx::PgPool, | |
| 92 | + | slug: &str, | |
| 93 | + | resource_id_str: &str, | |
| 94 | + | ) -> Result<Self, Response> { | |
| 95 | + | let community = get_community(db, slug).await?; | |
| 96 | + | let id = parse_uuid(resource_id_str)?; | |
| 97 | + | let resource = T::load(db, id) | |
| 98 | + | .await | |
| 99 | + | .map_err(|e| { | |
| 100 | + | tracing::error!(error = ?e, "db error loading scoped resource"); | |
| 101 | + | crate::error_page::internal_error() | |
| 102 | + | })? | |
| 103 | + | .ok_or_else(crate::error_page::not_found)?; | |
| 104 | + | if resource.community_id() != community.id { | |
| 105 | + | // The resource exists but lives in a different community than the | |
| 106 | + | // slug names — indistinguishable from "not found" to this URL. | |
| 107 | + | return Err(crate::error_page::not_found()); | |
| 108 | + | } | |
| 109 | + | Ok(Self { community, resource }) | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | /// Enforce write access (suspension/ban/mute) against the verified community. | |
| 113 | + | #[allow(clippy::result_large_err)] | |
| 114 | + | pub(crate) async fn require_write_access( | |
| 115 | + | &self, | |
| 116 | + | db: &sqlx::PgPool, | |
| 117 | + | user_id: Uuid, | |
| 118 | + | ) -> Result<(), Response> { | |
| 119 | + | check_write_access(db, self.community.id, user_id, self.community.suspended_at.is_some()) | |
| 120 | + | .await | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | /// The caller's role in the verified community. | |
| 124 | + | #[allow(clippy::result_large_err)] | |
| 125 | + | pub(crate) async fn role( | |
| 126 | + | &self, | |
| 127 | + | db: &sqlx::PgPool, | |
| 128 | + | user_id: Uuid, | |
| 129 | + | ) -> Result<Option<CommunityRole>, Response> { | |
| 130 | + | get_role(db, user_id, self.community.id).await | |
| 131 | + | } | |
| 132 | + | } |
| @@ -202,7 +202,9 @@ async fn mod_can_remove_post() { | |||
| 202 | 202 | resp.status | |
| 203 | 203 | ); | |
| 204 | 204 | ||
| 205 | - | // Verify removed_at is set but content preserved | |
| 205 | + | // Verify removed_at is set but content preserved. Direct loader use is fine | |
| 206 | + | // in a test assertion (no community-scope invariant to uphold here). | |
| 207 | + | #[allow(clippy::disallowed_methods)] | |
| 206 | 208 | let post_data = mt_db::queries::get_post_for_edit(&h.db, post_id) | |
| 207 | 209 | .await | |
| 208 | 210 | .unwrap() |
| @@ -574,3 +574,46 @@ async fn removing_reply_does_not_cascade() { | |||
| 574 | 574 | let resp = h.client.get(&thread_url).await; | |
| 575 | 575 | assert_eq!(resp.status.as_u16(), 200, "Thread should still be viewable"); | |
| 576 | 576 | } | |
| 577 | + | ||
| 578 | + | /// Regression (ultra-fuzz CHRONIC C1): a thread can only be acted on through its | |
| 579 | + | /// own community's slug. CommunityScope::resolve 404s when the slug names a | |
| 580 | + | /// different community than the resource lives in — even for a user who is a | |
| 581 | + | /// moderator/owner in the slug's community, so role can't paper over the | |
| 582 | + | /// mismatch. This is the structural seal the per-site guard kept failing to hold. | |
| 583 | + | #[tokio::test] | |
| 584 | + | async fn delete_thread_via_wrong_community_slug_404s() { | |
| 585 | + | let mut h = TestHarness::new().await; | |
| 586 | + | let actor = h.login_as("c1actor").await; | |
| 587 | + | ||
| 588 | + | let alpha = h.create_community("Alpha", "alpha").await; | |
| 589 | + | let alpha_cat = h.create_category(alpha, "General", "general").await; | |
| 590 | + | h.add_membership(actor, alpha, "moderator").await; | |
| 591 | + | ||
| 592 | + | // The actor is an OWNER in Beta, so a role check against Beta would pass — | |
| 593 | + | // only the resource-in-community check can stop this. | |
| 594 | + | let beta = h.create_community("Beta", "beta").await; | |
| 595 | + | let _beta_cat = h.create_category(beta, "General", "general").await; | |
| 596 | + | h.add_membership(actor, beta, "owner").await; | |
| 597 | + | ||
| 598 | + | let thread = h | |
| 599 | + | .create_thread_with_post(alpha_cat, actor, "Lives in Alpha", "Body") | |
| 600 | + | .await; | |
| 601 | + | ||
| 602 | + | // Attempt to delete Alpha's thread through Beta's slug. | |
| 603 | + | h.client.get("/p/beta/general").await; | |
| 604 | + | let url = format!("/p/beta/general/{}/delete", thread); | |
| 605 | + | let resp = h.client.post_form(&url, "").await; | |
| 606 | + | assert_eq!( | |
| 607 | + | resp.status.as_u16(), | |
| 608 | + | 404, | |
| 609 | + | "a thread acted on through the wrong community's slug must 404" | |
| 610 | + | ); | |
| 611 | + | ||
| 612 | + | // Alpha's thread is untouched. | |
| 613 | + | let deleted: bool = sqlx::query_scalar("SELECT deleted_at IS NOT NULL FROM threads WHERE id = $1") | |
| 614 | + | .bind(thread) | |
| 615 | + | .fetch_one(&h.db) | |
| 616 | + | .await | |
| 617 | + | .unwrap(); | |
| 618 | + | assert!(!deleted, "thread must not be deleted via a mismatched community slug"); | |
| 619 | + | } |