max / makenotwork
8 files changed,
+140 insertions,
-58 deletions
| @@ -181,12 +181,15 @@ pub async fn fetch_og_metadata( | |||
| 181 | 181 | return None; | |
| 182 | 182 | } | |
| 183 | 183 | ||
| 184 | - | // Only fetch HTML content | |
| 185 | - | if let Some(ct) = resp.headers().get(CONTENT_TYPE) { | |
| 186 | - | let ct_str = ct.to_str().unwrap_or(""); | |
| 187 | - | if !ct_str.starts_with("text/html") { | |
| 188 | - | return None; | |
| 189 | - | } | |
| 184 | + | // Only parse HTML content. Default-deny: a response with no Content-Type | |
| 185 | + | // (or an unreadable one) is not treated as HTML. | |
| 186 | + | let is_html = resp | |
| 187 | + | .headers() | |
| 188 | + | .get(CONTENT_TYPE) | |
| 189 | + | .and_then(|ct| ct.to_str().ok()) | |
| 190 | + | .is_some_and(|ct| ct.starts_with("text/html")); | |
| 191 | + | if !is_html { | |
| 192 | + | return None; | |
| 190 | 193 | } | |
| 191 | 194 | ||
| 192 | 195 | // Read body in chunks, capping at MAX_BODY_SIZE |
| @@ -121,7 +121,7 @@ async fn main() { | |||
| 121 | 121 | .layer(tower_http::set_header::SetResponseHeaderLayer::overriding( | |
| 122 | 122 | axum::http::header::CONTENT_SECURITY_POLICY, | |
| 123 | 123 | axum::http::HeaderValue::from_static( | |
| 124 | - | "default-src 'self'; img-src 'self'; style-src 'self' 'unsafe-inline'; frame-ancestors 'none'", | |
| 124 | + | "default-src 'self'; img-src 'self'; style-src 'self'; frame-ancestors 'none'", | |
| 125 | 125 | ), | |
| 126 | 126 | )) | |
| 127 | 127 | .layer(tower_http::set_header::SetResponseHeaderLayer::overriding( |
| @@ -191,8 +191,14 @@ pub(in crate::routes) async fn toggle_endorsement_handler( | |||
| 191 | 191 | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 192 | 192 | })?; | |
| 193 | 193 | ||
| 194 | - | // Check community access (suspension + ban) — no mute check since endorsing is not content | |
| 194 | + | // Check community access (suspension + ban) — no mute check since endorsing is not content. | |
| 195 | + | // Authorize against the post's *own* community, not just the URL slug, so a | |
| 196 | + | // user banned in the post's community can't route the endorsement through a | |
| 197 | + | // different slug to evade the ban check (matches the flag/remove paths). | |
| 195 | 198 | let community = get_community(&state.db, &slug).await?; | |
| 199 | + | if post_data.community_id != community.id { | |
| 200 | + | return Err((StatusCode::NOT_FOUND, "Not found").into_response()); | |
| 201 | + | } | |
| 196 | 202 | check_community_access(&state.db, &community, Some(user.user_id)).await?; | |
| 197 | 203 | // Endorsement is a write action, so it's blocked by Frozen/Archived state. | |
| 198 | 204 | check_write_state(&state, &community, &user, WriteScope::ContinueExisting).await?; |
| @@ -183,18 +183,6 @@ pub(in crate::routes) async fn category( | |||
| 183 | 183 | MaybeUser(session_user): MaybeUser, | |
| 184 | 184 | ) -> Result<impl IntoResponse, Response> { | |
| 185 | 185 | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 186 | - | let community = get_community(&state.db, &slug).await?; | |
| 187 | - | ||
| 188 | - | check_community_access(&state.db, &community, session_user.as_ref().map(|u| u.user_id)).await?; | |
| 189 | - | ||
| 190 | - | let cat = mt_db::queries::get_category_by_slugs(&state.db, &slug, &category_slug) | |
| 191 | - | .await | |
| 192 | - | .map_err(|e| { | |
| 193 | - | tracing::error!(error = ?e, "db error fetching category"); | |
| 194 | - | crate::error_page::internal_error() | |
| 195 | - | })? | |
| 196 | - | .ok_or_else(crate::error_page::not_found)?; | |
| 197 | - | ||
| 198 | 186 | let per_page: i64 = 25; | |
| 199 | 187 | ||
| 200 | 188 | // Parse sort column — only allow known values, default to "activity" | |
| @@ -203,14 +191,33 @@ pub(in crate::routes) async fn category( | |||
| 203 | 191 | ||
| 204 | 192 | let tag_filter = query.tag.as_deref().filter(|t| !t.is_empty()); | |
| 205 | 193 | ||
| 206 | - | let total = mt_db::queries::count_threads_in_category_filtered( | |
| 207 | - | &state.db, &slug, &category_slug, tag_filter, | |
| 208 | - | ) | |
| 209 | - | .await | |
| 210 | - | .map_err(|e| { | |
| 211 | - | tracing::error!(error = ?e, "db error counting threads"); | |
| 212 | - | crate::error_page::internal_error() | |
| 213 | - | })?; | |
| 194 | + | // Community, category, and the thread count are all keyed off the slugs and | |
| 195 | + | // independent of each other — run them concurrently rather than as three | |
| 196 | + | // serial round-trips (matches thread.rs's batched fetch). | |
| 197 | + | let (community, cat, total) = tokio::try_join!( | |
| 198 | + | async { get_community(&state.db, &slug).await }, | |
| 199 | + | async { | |
| 200 | + | mt_db::queries::get_category_by_slugs(&state.db, &slug, &category_slug) | |
| 201 | + | .await | |
| 202 | + | .map_err(|e| { | |
| 203 | + | tracing::error!(error = ?e, "db error fetching category"); | |
| 204 | + | crate::error_page::internal_error() | |
| 205 | + | })? | |
| 206 | + | .ok_or_else(crate::error_page::not_found) | |
| 207 | + | }, | |
| 208 | + | async { | |
| 209 | + | mt_db::queries::count_threads_in_category_filtered( | |
| 210 | + | &state.db, &slug, &category_slug, tag_filter, | |
| 211 | + | ) | |
| 212 | + | .await | |
| 213 | + | .map_err(|e| { | |
| 214 | + | tracing::error!(error = ?e, "db error counting threads"); | |
| 215 | + | crate::error_page::internal_error() | |
| 216 | + | }) | |
| 217 | + | }, | |
| 218 | + | )?; | |
| 219 | + | ||
| 220 | + | check_community_access(&state.db, &community, session_user.as_ref().map(|u| u.user_id)).await?; | |
| 214 | 221 | ||
| 215 | 222 | let pagination = Pagination::new(query.page.unwrap_or(1).max(1), total, per_page); | |
| 216 | 223 | let offset = pagination.offset(per_page); | |
| @@ -224,14 +231,45 @@ pub(in crate::routes) async fn category( | |||
| 224 | 231 | crate::error_page::internal_error() | |
| 225 | 232 | })?; | |
| 226 | 233 | ||
| 227 | - | // Batch-fetch tags for all threads | |
| 234 | + | // Per-thread tags, the user's mention status, and the community's tag list | |
| 235 | + | // are all independent given the thread ids (and community id) — fetch them | |
| 236 | + | // concurrently instead of three serial round-trips. | |
| 228 | 237 | let thread_ids: Vec<uuid::Uuid> = db_threads.iter().map(|t| t.id).collect(); | |
| 229 | - | let all_thread_tags = mt_db::queries::list_tags_for_threads(&state.db, &thread_ids) | |
| 230 | - | .await | |
| 231 | - | .map_err(|e| { | |
| 232 | - | tracing::error!(error = ?e, "db error fetching thread tags"); | |
| 233 | - | crate::error_page::internal_error() | |
| 234 | - | })?; | |
| 238 | + | let user_id = session_user.as_ref().map(|u| u.user_id); | |
| 239 | + | let (all_thread_tags, mention_thread_ids, db_tags) = tokio::try_join!( | |
| 240 | + | async { | |
| 241 | + | mt_db::queries::list_tags_for_threads(&state.db, &thread_ids) | |
| 242 | + | .await | |
| 243 | + | .map_err(|e| { | |
| 244 | + | tracing::error!(error = ?e, "db error fetching thread tags"); | |
| 245 | + | crate::error_page::internal_error() | |
| 246 | + | }) | |
| 247 | + | }, | |
| 248 | + | async { | |
| 249 | + | // Mention status for the logged-in user; a query error degrades to | |
| 250 | + | // "no mentions" rather than failing the page (as before). | |
| 251 | + | let set: std::collections::HashSet<String> = match user_id { | |
| 252 | + | Some(uid) => { | |
| 253 | + | mt_db::queries::get_threads_with_mentions_for_user(&state.db, uid, &thread_ids) | |
| 254 | + | .await | |
| 255 | + | .unwrap_or_default() | |
| 256 | + | .into_iter() | |
| 257 | + | .map(|id| id.to_string()) | |
| 258 | + | .collect() | |
| 259 | + | } | |
| 260 | + | None => std::collections::HashSet::new(), | |
| 261 | + | }; | |
| 262 | + | Ok::<_, Response>(set) | |
| 263 | + | }, | |
| 264 | + | async { | |
| 265 | + | mt_db::queries::list_tags_for_community(&state.db, community.id) | |
| 266 | + | .await | |
| 267 | + | .map_err(|e| { | |
| 268 | + | tracing::error!(error = ?e, "db error listing tags"); | |
| 269 | + | crate::error_page::internal_error() | |
| 270 | + | }) | |
| 271 | + | }, | |
| 272 | + | )?; | |
| 235 | 273 | ||
| 236 | 274 | let mut tags_by_thread: HashMap<String, Vec<TagBadge>> = HashMap::new(); | |
| 237 | 275 | for tt in all_thread_tags { | |
| @@ -241,18 +279,6 @@ pub(in crate::routes) async fn category( | |||
| 241 | 279 | .push(TagBadge { id: String::new(), name: tt.tag_name, slug: tt.tag_slug }); | |
| 242 | 280 | } | |
| 243 | 281 | ||
| 244 | - | // Batch-fetch mention status for logged-in user | |
| 245 | - | let mention_thread_ids: std::collections::HashSet<String> = if let Some(ref user) = session_user { | |
| 246 | - | mt_db::queries::get_threads_with_mentions_for_user(&state.db, user.user_id, &thread_ids) | |
| 247 | - | .await | |
| 248 | - | .unwrap_or_default() | |
| 249 | - | .into_iter() | |
| 250 | - | .map(|id| id.to_string()) | |
| 251 | - | .collect() | |
| 252 | - | } else { | |
| 253 | - | std::collections::HashSet::new() | |
| 254 | - | }; | |
| 255 | - | ||
| 256 | 282 | let threads = db_threads | |
| 257 | 283 | .into_iter() | |
| 258 | 284 | .map(|t| { | |
| @@ -274,13 +300,6 @@ pub(in crate::routes) async fn category( | |||
| 274 | 300 | }) | |
| 275 | 301 | .collect(); | |
| 276 | 302 | ||
| 277 | - | // Load available tags for filter UI | |
| 278 | - | let db_tags = mt_db::queries::list_tags_for_community(&state.db, community.id) | |
| 279 | - | .await | |
| 280 | - | .map_err(|e| { | |
| 281 | - | tracing::error!(error = ?e, "db error listing tags"); | |
| 282 | - | crate::error_page::internal_error() | |
| 283 | - | })?; | |
| 284 | 303 | let available_tags = db_tags | |
| 285 | 304 | .into_iter() | |
| 286 | 305 | .map(|t| TagBadge { id: t.id.to_string(), name: t.name, slug: t.slug }) |
| @@ -206,7 +206,9 @@ pub(crate) fn field_error(field: &'static str, message: impl Into<String>) -> Re | |||
| 206 | 206 | #[allow(clippy::result_large_err)] | |
| 207 | 207 | pub(crate) fn validate_title(text: &str) -> Result<&str, Response> { | |
| 208 | 208 | let t = text.trim(); | |
| 209 | - | if t.is_empty() || t.len() > 256 { | |
| 209 | + | // Count characters, not bytes, to match the client `maxlength` (UTF-16 | |
| 210 | + | // units) — else a multibyte title under the visible cap fails server-side. | |
| 211 | + | if t.is_empty() || t.chars().count() > 256 { | |
| 210 | 212 | return Err(field_error( | |
| 211 | 213 | "title", | |
| 212 | 214 | "Title must be between 1 and 256 characters.", | |
| @@ -221,7 +223,8 @@ pub(crate) fn validate_title(text: &str) -> Result<&str, Response> { | |||
| 221 | 223 | #[allow(clippy::result_large_err)] | |
| 222 | 224 | pub(crate) fn validate_body<'a>(text: &'a str, max: usize, label: &str) -> Result<&'a str, Response> { | |
| 223 | 225 | let t = text.trim(); | |
| 224 | - | if t.is_empty() || t.len() > max { | |
| 226 | + | // Character count (not byte length) to match the client-side cap. | |
| 227 | + | if t.is_empty() || t.chars().count() > max { | |
| 225 | 228 | return Err(field_error( | |
| 226 | 229 | "body", | |
| 227 | 230 | format!("{label} must be between 1 and {max} characters."), | |
| @@ -538,6 +541,17 @@ mod validation_tests { | |||
| 538 | 541 | assert!(validate_title(&s).is_err()); | |
| 539 | 542 | } | |
| 540 | 543 | ||
| 544 | + | #[test] | |
| 545 | + | fn title_counts_characters_not_bytes() { | |
| 546 | + | // 256 multibyte chars = 1024 bytes. Must pass (char count), matching the | |
| 547 | + | // client `maxlength`; a byte-length check would wrongly reject it. | |
| 548 | + | let s = "é".repeat(256); | |
| 549 | + | assert!(validate_title(&s).is_ok(), "256 multibyte chars must pass"); | |
| 550 | + | // 257 of them must still reject on character count. | |
| 551 | + | let s = "é".repeat(257); | |
| 552 | + | assert!(validate_title(&s).is_err(), "257 chars must reject"); | |
| 553 | + | } | |
| 554 | + | ||
| 541 | 555 | // ── validate_body (1..=max chars after trim) ── | |
| 542 | 556 | ||
| 543 | 557 | #[test] |
| @@ -304,7 +304,9 @@ document.addEventListener('submit', function(e) { | |||
| 304 | 304 | (function() { | |
| 305 | 305 | var body = document.getElementById('body') || document.getElementById('reply-body'); | |
| 306 | 306 | if (!body || body.tagName !== 'TEXTAREA') return; | |
| 307 | - | if (localStorage.getItem('mt_tracking_enabled') === 'false') return; | |
| 307 | + | // NB: draft autosave is independent of the read-position tracking opt-out | |
| 308 | + | // (mt_tracking_enabled). They share no behavior — gating drafts on the | |
| 309 | + | // privacy toggle silently dropped a long post on a validation error. | |
| 308 | 310 | ||
| 309 | 311 | var title = document.getElementById('title'); | |
| 310 | 312 | if (title && title.tagName !== 'INPUT') title = null; |
| @@ -17,7 +17,7 @@ | |||
| 17 | 17 | <a href="/account">Account</a> | |
| 18 | 18 | <a href="{{ mnw_base_url }}/dashboard">Dashboard</a> | |
| 19 | 19 | {% if user.is_platform_admin %}<a href="/_admin">Admin</a>{% endif %} | |
| 20 | - | <form method="post" action="/auth/logout" style="display:inline"><button type="submit" class="link-button" aria-label="Log out">Log Out</button></form> | |
| 20 | + | <form method="post" action="/auth/logout" class="form-inline"><button type="submit" class="link-button" aria-label="Log out">Log Out</button></form> | |
| 21 | 21 | {% else %} | |
| 22 | 22 | <a href="/auth/login">Login</a> | |
| 23 | 23 | <a href="{{ mnw_base_url }}/join">Join</a> |
| @@ -44,6 +44,44 @@ async fn endorse_post_happy_path() { | |||
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | 46 | #[tokio::test] | |
| 47 | + | async fn endorse_via_mismatched_community_slug_is_404() { | |
| 48 | + | // A post lives in community A; routing the endorse through community B's | |
| 49 | + | // slug must 404 (post.community_id != slug-derived community), so a user | |
| 50 | + | // banned in A can't evade the ban check via another slug (audit S-2). | |
| 51 | + | let mut h = TestHarness::new().await; | |
| 52 | + | let author_id = h.login_as("author").await; | |
| 53 | + | let comm_a = h.create_community("Alpha", "alpha").await; | |
| 54 | + | let cat_a = h.create_category(comm_a, "General", "general").await; | |
| 55 | + | h.add_membership(author_id, comm_a, "member").await; | |
| 56 | + | // A second, unrelated community the attacker is a member of. | |
| 57 | + | let comm_b = h.create_community("Beta", "beta").await; | |
| 58 | + | h.create_category(comm_b, "General", "general").await; | |
| 59 | + | ||
| 60 | + | let thread_id = h | |
| 61 | + | .create_thread_with_post(cat_a, author_id, "Post in A", "Content") | |
| 62 | + | .await; | |
| 63 | + | let post_id = mt_db::queries::list_posts_in_thread(&h.db, thread_id) | |
| 64 | + | .await | |
| 65 | + | .unwrap()[0] | |
| 66 | + | .id; | |
| 67 | + | ||
| 68 | + | let endorser_id = h.login_as("endorser").await; | |
| 69 | + | h.add_membership(endorser_id, comm_b, "member").await; | |
| 70 | + | h.client.get(&format!("/p/alpha/general/{thread_id}")).await; // CSRF | |
| 71 | + | ||
| 72 | + | // Use community B's slug with A's thread/post ids. | |
| 73 | + | let url = format!("/p/beta/general/{thread_id}/posts/{post_id}/endorse"); | |
| 74 | + | let resp = h.client.post_form(&url, "").await; | |
| 75 | + | assert_eq!(resp.status.as_u16(), 404, "mismatched-slug endorse must 404"); | |
| 76 | + | ||
| 77 | + | // And no endorsement was recorded. | |
| 78 | + | let endorsements = mt_db::queries::list_endorsements_for_posts(&h.db, &[post_id]) | |
| 79 | + | .await | |
| 80 | + | .unwrap(); | |
| 81 | + | assert!(endorsements.is_empty(), "no endorsement should be recorded"); | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | #[tokio::test] | |
| 47 | 85 | async fn toggle_endorsement_removes() { | |
| 48 | 86 | let mut h = TestHarness::new().await; | |
| 49 | 87 | let author_id = h.login_as("author2").await; |