From d92a273797bdd9e7e75f67e4d4810b1dc853bb84 Mon Sep 17 00:00:00 2001 From: Leo Torres Date: Fri, 27 Mar 2026 11:58:32 +0100 Subject: [PATCH 1/2] fix: prevent race condition in private annotation note creation Use SELECT FOR UPDATE on the annotation row to serialize concurrent requests, preventing two requests from both passing the max-1-note check and inserting duplicate notes on private annotations. Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/aris/routes/file_annotations.py | 11 +++++-- .../test_routes_file_annotations.py | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/backend/aris/routes/file_annotations.py b/backend/aris/routes/file_annotations.py index 78ecf2a3..b40398f8 100644 --- a/backend/aris/routes/file_annotations.py +++ b/backend/aris/routes/file_annotations.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, Depends, HTTPException, status from pydantic import BaseModel, ConfigDict -from sqlalchemy import and_, or_, select +from sqlalchemy import and_, func, or_, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -274,15 +274,20 @@ async def create_annotation_message( raise HTTPException(status_code=403, detail="Comment permission required") # Private annotations: single note only. Shared annotations: unlimited (thread). + # Lock the annotation row to prevent race conditions where two concurrent + # requests both pass the count check before either inserts. if annotation.visibility != AnnotationVisibility.SHARED: - count_query = select(AnnotationMessage).where( + await db.execute( + select(Annotation).where(Annotation.id == annotation_id).with_for_update() + ) + count_query = select(func.count()).select_from(AnnotationMessage).where( and_( AnnotationMessage.annotation_id == annotation_id, AnnotationMessage.deleted_at.is_(None), ) ) count_result = await db.execute(count_query) - if len(count_result.scalars().all()) >= 1: + if count_result.scalar() >= 1: raise HTTPException( status_code=400, detail="Annotation already has a note" ) diff --git a/backend/tests/test_routes/test_routes_file_annotations.py b/backend/tests/test_routes/test_routes_file_annotations.py index 826e017c..41166803 100644 --- a/backend/tests/test_routes/test_routes_file_annotations.py +++ b/backend/tests/test_routes/test_routes_file_annotations.py @@ -444,6 +444,35 @@ async def test_private_annotation_single_note_limit( assert "already has a note" in resp2.json()["detail"] +async def test_private_annotation_note_limit_after_delete_and_recreate( + client: AsyncClient, authenticated_user +): + """After deleting a note, a new one can be created (soft-delete aware).""" + headers = {"Authorization": f"Bearer {authenticated_user['token']}"} + file_id = await _create_file(client, headers, authenticated_user["user_id"]) + ann = await _create_annotation(client, headers, file_id) + + resp1 = await client.post( + f"/annotations/{ann['id']}/messages", + headers=headers, + json={"content": "Original note"}, + ) + assert resp1.status_code == 201 + msg_id = resp1.json()["id"] + + delete_resp = await client.delete( + f"/annotations/messages/{msg_id}", headers=headers + ) + assert delete_resp.status_code == 204 + + resp2 = await client.post( + f"/annotations/{ann['id']}/messages", + headers=headers, + json={"content": "Replacement note"}, + ) + assert resp2.status_code == 201 + + async def test_shared_annotation_multiple_notes( client: AsyncClient, authenticated_user, From 1d2cf80505d7bb59cd332f4c88a1c6666c660432 Mon Sep 17 00:00:00 2001 From: Leo Torres Date: Fri, 27 Mar 2026 12:17:07 +0100 Subject: [PATCH 2/2] fix: use scalar_one() to fix mypy type error in annotation note count check scalar() returns Optional[int], causing mypy error when compared with >= 1. scalar_one() is safe here since COUNT() always returns exactly one row. Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/aris/routes/file_annotations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/aris/routes/file_annotations.py b/backend/aris/routes/file_annotations.py index b40398f8..2a5ce9cd 100644 --- a/backend/aris/routes/file_annotations.py +++ b/backend/aris/routes/file_annotations.py @@ -287,7 +287,7 @@ async def create_annotation_message( ) ) count_result = await db.execute(count_query) - if count_result.scalar() >= 1: + if count_result.scalar_one() >= 1: raise HTTPException( status_code=400, detail="Annotation already has a note" )