From f3827706dff45d440288d033d16916c7bebe00da Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 27 Apr 2026 05:11:39 -0400 Subject: [PATCH] fix: drawer delete_turn guards event_id <= 0 (T110.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stale tab or hand-crafted request posting event_id=0 to the surgical delete route would compute after_event_id=-1 and silently truncate the entire log. Now rejected with 400. SQLite assigns event_log ids starting at 1, so any legitimate id is always >= 1 — non-positive values can only indicate a client bug. Test: tests/test_drawer_phase4.py::test_delete_turn_with_event_id_zero_returns_400. --- chat/web/drawer.py | 12 ++++++++++++ tests/test_drawer_phase4.py | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/chat/web/drawer.py b/chat/web/drawer.py index 5396ae8..f0c3ddb 100644 --- a/chat/web/drawer.py +++ b/chat/web/drawer.py @@ -1278,7 +1278,19 @@ async def delete_turn( A snapshot is taken before truncation (inside ``execute_rewind``) so the user can recover via the snapshot index. + + T110.1 guards ``event_id <= 0``: a stale tab or hand-crafted request + posting ``event_id=0`` would otherwise compute ``after_event_id=-1`` + and silently truncate the entire log. ``id`` is auto-assigned by + SQLite starting at 1 so any caller's "real" id is always >= 1; a + zero or negative value can only mean a client bug, surfaced as 400. """ + if int(event_id) <= 0: + raise HTTPException( + status_code=400, + detail=f"event_id must be a positive integer, got {event_id}", + ) + chat = get_chat(conn, chat_id) if chat is None: raise HTTPException(status_code=404, detail=f"chat not found: {chat_id}") diff --git a/tests/test_drawer_phase4.py b/tests/test_drawer_phase4.py index f94f266..f4f3235 100644 --- a/tests/test_drawer_phase4.py +++ b/tests/test_drawer_phase4.py @@ -458,6 +458,29 @@ def test_t98_4_delete_invokes_rewind_and_drops_cascade(client, tmp_path): assert row is None, f"event {ev_id} should have been deleted" +def test_delete_turn_with_event_id_zero_returns_400(client, tmp_path): + """T110.1: ``event_id <= 0`` is an obvious client error and must NOT + silently rewind the entire log via ``after_event_id = -1``. The route + rejects it with 400 so the audit trail stays intact. + """ + db = tmp_path / "test.db" + _seed_chat(db) + _seed_turns(db) + + # Sanity: events present before the bad request. + with open_db(db) as conn: + before = conn.execute("SELECT COUNT(*) FROM event_log").fetchone()[0] + assert before > 0 + + response = client.post("/chats/chat_bot_a/drawer/turn/delete/0") + assert response.status_code == 400 + + # And the log was NOT truncated. + with open_db(db) as conn: + after = conn.execute("SELECT COUNT(*) FROM event_log").fetchone()[0] + assert after == before + + # --------------------------------------------------------------------------- # T98.5 — remaining v1 edits (chat narrative anchor + weather). # ---------------------------------------------------------------------------