Skip to main content

max / makenotwork

mt: make reply insert conditional on a live, unlocked thread create_reply_handler read locked/deleted_at from the resolve-time snapshot, then inserted later with no in-tx re-check: a concurrent lock or soft-delete in the gap let the reply commit and the m029 post_count trigger increment a dead thread. create_post_tx now inserts via SELECT ... WHERE EXISTS (thread live AND unlocked), returning None when the thread changed under it; the handler surfaces that as 409. Mirrors mod_remove_post_cascade's conditional write. The pool wrapper maps None to RowNotFound so its callers are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-07 15:06 UTC
Commit: b8c7acd2f72a6fa50ec8ea656a4a13f8eb5f83cd
Parent: 06ee048
4 files changed, +130 insertions, -8 deletions
A multithreaded/.sqlx/query-6db83698c572a4de18bfe97c5c3641c72f9359f1839c92975311972d10def6c7.json +25
@@ -0,0 +1,25 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "INSERT INTO posts (thread_id, author_id, body_markdown, body_html)\n SELECT $1, $2, $3, $4\n WHERE EXISTS (\n SELECT 1 FROM threads\n WHERE id = $1 AND deleted_at IS NULL AND locked = false\n )\n RETURNING id",
4 + "describe": {
5 + "columns": [
6 + {
7 + "ordinal": 0,
8 + "name": "id",
9 + "type_info": "Uuid"
10 + }
11 + ],
12 + "parameters": {
13 + "Left": [
14 + "Uuid",
15 + "Uuid",
16 + "Text",
17 + "Text"
18 + ]
19 + },
20 + "nullable": [
21 + false
22 + ]
23 + },
24 + "hash": "6db83698c572a4de18bfe97c5c3641c72f9359f1839c92975311972d10def6c7"
25 + }
M multithreaded/crates/mt-db/src/mutations.rs +26 -7
@@ -278,7 +278,12 @@ pub async fn create_post(
278 278 body_html: &str,
279 279 ) -> Result<Uuid, sqlx::Error> {
280 280 let mut tx = pool.begin().await?;
281 - let post_id = create_post_tx(&mut tx, thread_id, author_id, body_markdown, body_html).await?;
281 + let post_id = create_post_tx(&mut tx, thread_id, author_id, body_markdown, body_html)
282 + .await?
283 + // The pool-taking wrapper is for callers that have already established the
284 + // thread is live (tests, simple internal paths); a locked/deleted thread
285 + // here is a "row not inserted" error rather than a silent no-op.
286 + .ok_or(sqlx::Error::RowNotFound)?;
282 287 tx.commit().await?;
283 288 Ok(post_id)
284 289 }
@@ -296,22 +301,36 @@ pub async fn create_post_tx(
296 301 author_id: Uuid,
297 302 body_markdown: &str,
298 303 body_html: &str,
299 - ) -> Result<Uuid, sqlx::Error> {
304 + ) -> Result<Option<Uuid>, sqlx::Error> {
305 + // Insert only while the thread is still live and unlocked, in the same
306 + // statement that reads that state — closing the TOCTOU between a handler's
307 + // snapshot-time `locked`/`deleted_at` check and this insert. Without the
308 + // guard a concurrent lock or soft-delete in the gap would let the reply
309 + // commit (and the m029 post_count trigger increment a deleted thread). None
310 + // = the thread was locked/deleted meanwhile; the caller surfaces it. This
311 + // mirrors the conditional-write pattern `mod_remove_post_cascade` uses.
300 312 let post_id = sqlx::query_scalar!(
301 313 "INSERT INTO posts (thread_id, author_id, body_markdown, body_html)
302 - VALUES ($1, $2, $3, $4)
314 + SELECT $1, $2, $3, $4
315 + WHERE EXISTS (
316 + SELECT 1 FROM threads
317 + WHERE id = $1 AND deleted_at IS NULL AND locked = false
318 + )
303 319 RETURNING id",
304 320 thread_id,
305 321 author_id,
306 322 body_markdown,
307 323 body_html,
308 324 )
309 - .fetch_one(&mut *conn)
325 + .fetch_optional(&mut *conn)
310 326 .await?;
311 327
312 - sqlx::query!("UPDATE threads SET last_activity_at = now() WHERE id = $1", thread_id)
313 - .execute(&mut *conn)
314 - .await?;
328 + // Only bump activity if the reply actually landed.
329 + if post_id.is_some() {
330 + sqlx::query!("UPDATE threads SET last_activity_at = now() WHERE id = $1", thread_id)
331 + .execute(&mut *conn)
332 + .await?;
333 + }
315 334
316 335 Ok(post_id)
317 336 }
M multithreaded/src/routes/forum/posts.rs +8 -1
@@ -342,9 +342,16 @@ pub(in crate::routes) async fn create_reply_handler(
342 342 // Create the reply and its @mentions as one atomic unit so a failure between
343 343 // them can't leave a committed reply whose mentions silently vanished (M-St1).
344 344 let mut tx = begin_tx(&state.db).await?;
345 + // Conditional insert: lands only if the thread is still live+unlocked, closing
346 + // the TOCTOU against the snapshot-time `locked` check above. None => a
347 + // concurrent lock/soft-delete won the race.
345 348 let post_id = mt_db::mutations::create_post_tx(&mut tx, thread_id, user.user_id, body, &body_html)
346 349 .await
347 - .map_err(db_error)?;
350 + .map_err(db_error)?
351 + .ok_or_else(|| {
352 + (StatusCode::CONFLICT, "This thread was locked or removed before your reply posted.")
353 + .into_response()
354 + })?;
348 355 mt_db::mutations::insert_mentions(&mut *tx, post_id, &mention_ids)
349 356 .await
350 357 .map_err(db_error)?;
M multithreaded/tests/workflows/mutations.rs +71
@@ -415,6 +415,77 @@ async fn create_post_bumps_thread_activity() {
415 415 assert!(recent, "last_activity_at should be updated to recent time");
416 416 }
417 417
418 + /// Regression (fuzz-2026-07-06 TOCTOU): a reply insert is conditional on the
419 + /// thread still being live and unlocked, in the same statement — so a lock or
420 + /// soft-delete that races the handler's snapshot check can't let the reply
421 + /// commit (nor let the post_count trigger increment a dead thread). Exercised at
422 + /// the DB layer, where the guard lives.
423 + #[tokio::test]
424 + async fn reply_insert_blocked_on_locked_or_deleted_thread() {
425 + let h = TestHarness::new().await;
426 + let comm_id = h.create_community("Test", "test").await;
427 + let cat_id = h.create_category(comm_id, "General", "general").await;
428 +
429 + let author = Uuid::new_v4();
430 + sqlx::query("INSERT INTO users (mnw_account_id, username) VALUES ($1, 'toctouauthor')")
431 + .bind(author)
432 + .execute(&h.db)
433 + .await
434 + .unwrap();
435 + let thread_id = mt_db::mutations::create_thread(&h.db, cat_id, author, "TOCTOU")
436 + .await
437 + .unwrap();
438 +
439 + // Locked → no insert.
440 + sqlx::query("UPDATE threads SET locked = true WHERE id = $1")
441 + .bind(thread_id)
442 + .execute(&h.db)
443 + .await
444 + .unwrap();
445 + assert!(
446 + matches!(
447 + mt_db::mutations::create_post(&h.db, thread_id, author, "r", "<p>r</p>").await,
448 + Err(sqlx::Error::RowNotFound)
449 + ),
450 + "a reply must not insert on a locked thread"
451 + );
452 +
453 + // Unlocked but soft-deleted → still no insert.
454 + sqlx::query("UPDATE threads SET locked = false, deleted_at = now() WHERE id = $1")
455 + .bind(thread_id)
456 + .execute(&h.db)
457 + .await
458 + .unwrap();
459 + assert!(
460 + matches!(
461 + mt_db::mutations::create_post(&h.db, thread_id, author, "r", "<p>r</p>").await,
462 + Err(sqlx::Error::RowNotFound)
463 + ),
464 + "a reply must not insert on a soft-deleted thread"
465 + );
466 +
467 + // Restored to live → the reply lands, proving the guard only blocks the bad states.
468 + sqlx::query("UPDATE threads SET deleted_at = NULL WHERE id = $1")
469 + .bind(thread_id)
470 + .execute(&h.db)
471 + .await
472 + .unwrap();
473 + assert!(
474 + mt_db::mutations::create_post(&h.db, thread_id, author, "r", "<p>r</p>")
475 + .await
476 + .is_ok(),
477 + "a live, unlocked thread must still accept the reply"
478 + );
479 +
480 + // The post_count trigger only counted the one successful insert.
481 + let post_count: i32 = sqlx::query_scalar("SELECT post_count FROM threads WHERE id = $1")
482 + .bind(thread_id)
483 + .fetch_one(&h.db)
484 + .await
485 + .unwrap();
486 + assert_eq!(post_count, 1, "only the successful reply may bump post_count");
487 + }
488 +
418 489 // ============================================================================
419 490 // Endorsement mutations
420 491 // ============================================================================