From b7979e2f9f9ae7f9190a14acaa9cd63ffb14e3ae Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Thu, 28 May 2026 20:26:15 -0400 Subject: [PATCH] fix(catalog): re-check orphan status when deleting library items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orphan detection moved outside the media_items delete in the batched library-delete rewrite, opening a TOCTOU race: a concurrent scan/import could attach one of the collected content IDs to another library between collectOrphanBatch and the delete, after which the unconditional `DELETE FROM media_items WHERE content_id = ANY($1)` would still remove the shared row and cascade away the newly-added membership — dropping the item from the other library. Re-check the orphan invariant inside the delete (NOT EXISTS a membership in another folder) and count rows actually deleted. Addresses PR #21 review (P1). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/catalog/folder_repo.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/internal/catalog/folder_repo.go b/internal/catalog/folder_repo.go index b5a11004..b1485308 100644 --- a/internal/catalog/folder_repo.go +++ b/internal/catalog/folder_repo.go @@ -534,13 +534,30 @@ func (r *FolderRepository) DeleteWithStats( for _, d := range dirs { rawDirs[d] = struct{}{} } + var deleted int64 if err := retryOnDeadlock(ctx, func() error { - _, e := r.pool.Exec(ctx, `DELETE FROM media_items WHERE content_id = ANY($1)`, ids) - return e + // Re-check the orphan invariant inside the delete. A concurrent + // scan/import may have attached one of these content IDs to another + // library after collectOrphanBatch returned; without this guard the + // cascade would delete the shared media_items row (and the + // newly-added membership), dropping the item from the other library. + tag, e := r.pool.Exec(ctx, ` + DELETE FROM media_items + WHERE content_id = ANY($1) + AND NOT EXISTS ( + SELECT 1 FROM media_item_libraries other + WHERE other.content_id = media_items.content_id + AND other.media_folder_id <> $2 + )`, ids, id) + if e != nil { + return e + } + deleted = tag.RowsAffected() + return nil }); err != nil { return nil, fmt.Errorf("deleting orphaned items: %w", err) } - stats.OrphanedItems += len(ids) + stats.OrphanedItems += int(deleted) if progress != nil { progress(stats.OrphanedItems, orphanTotal, "Deleting orphaned items") }