From 60bf47ede0d98f96aa7e786e9919a0576ede3486 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom@getoutline.com>
Date: Fri, 29 May 2026 20:47:18 -0400
Subject: [PATCH] fix: Prevent foreign key violation when permanently deleting
 a team (#12527)

The attachment cleanup loop used findAllInBatches, which advances an
OFFSET each iteration. Because the callback deletes each batch, the
remaining rows shift backwards and the advancing offset skips over them,
leaving attachments that still reference the team. team.destroy() then
failed with attachments_teamId_fkey.

Page from offset 0 until no attachments remain, and remove the now
redundant per-user attachment delete so the loop is the single
authoritative cleanup.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---
 server/commands/teamPermanentDeleter.test.ts | 41 ++++++++++++++++++++
 server/commands/teamPermanentDeleter.ts      | 36 ++++++++---------
 2 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/server/commands/teamPermanentDeleter.test.ts b/server/commands/teamPermanentDeleter.test.ts
index a604ecda9b..b1b5730427 100644
--- a/server/commands/teamPermanentDeleter.test.ts
+++ b/server/commands/teamPermanentDeleter.test.ts
@@ -139,6 +139,47 @@ describe("teamPermanentDeleter", () => {
     ).toEqual(0);
   });
 
+  it("should destroy attachments spanning multiple batches", async () => {
+    const team = await buildTeam({
+      deletedAt: subDays(new Date(), 90),
+    });
+    const user = await buildUser({ teamId: team.id });
+    const document = await buildDocument({
+      teamId: team.id,
+      userId: user.id,
+    });
+
+    // More than a single batch (batchLimit is 100) to ensure no attachments are
+    // skipped while deleting, which would otherwise leave rows referencing the
+    // team and cause a foreign key violation when the team is destroyed.
+    await Promise.all(
+      Array.from({ length: 150 }, () =>
+        buildAttachment({
+          teamId: team.id,
+          userId: user.id,
+          documentId: document.id,
+        })
+      )
+    );
+
+    await teamPermanentDeleter(team);
+
+    expect(
+      await Attachment.count({
+        where: {
+          teamId: team.id,
+        },
+      })
+    ).toEqual(0);
+    expect(
+      await Team.count({
+        where: {
+          id: team.id,
+        },
+      })
+    ).toEqual(0);
+  });
+
   it("should destroy imports", async () => {
     const team = await buildTeam({
       deletedAt: subDays(new Date(), 90),
diff --git a/server/commands/teamPermanentDeleter.ts b/server/commands/teamPermanentDeleter.ts
index f3c4335eb7..2011e5627d 100644
--- a/server/commands/teamPermanentDeleter.ts
+++ b/server/commands/teamPermanentDeleter.ts
@@ -40,21 +40,25 @@ async function teamPermanentDeleter(team: Team) {
   );
   const teamId = team.id;
 
-  await Attachment.findAllInBatches<Attachment>(
-    {
+  // Attachments are destroyed as individual instances (rather than a bulk
+  // delete) so the BeforeDestroy hook runs and removes the associated file from
+  // storage. We cannot use findAllInBatches with an advancing offset here –
+  // deleting a batch shifts the remaining rows backwards, so advancing the
+  // offset would skip records and leave attachments that still reference the
+  // team, causing a foreign key violation when the team itself is destroyed.
+  // Instead we repeatedly fetch and delete the first batch until none remain.
+  let attachments: Attachment[];
+  do {
+    attachments = await Attachment.findAll<Attachment>({
       where: {
         teamId,
       },
-      batchLimit: 100,
-    },
-    async (attachments, options) => {
+      limit: 100,
+    });
+
+    if (attachments.length > 0) {
       await sequelize.transaction(async (transaction) => {
-        Logger.info(
-          "commands",
-          `Deleting attachments ${options.offset} – ${
-            (options.offset || 0) + (options?.limit || 0)
-          }…`
-        );
+        Logger.info("commands", `Deleting ${attachments.length} attachments…`);
         await Promise.all(
           attachments.map((attachment) =>
             attachment.destroy({
@@ -64,7 +68,7 @@ async function teamPermanentDeleter(team: Team) {
         );
       });
     }
-  );
+  } while (attachments.length > 0);
 
   // Destroy user-relation models
   await User.findAllInBatches<User>(
@@ -85,14 +89,6 @@ async function teamPermanentDeleter(team: Team) {
           force: true,
           transaction,
         });
-        await Attachment.destroy({
-          where: {
-            teamId,
-            userId: userIds,
-          },
-          force: true,
-          transaction,
-        });
         await ApiKey.destroy({
           where: {
             userId: userIds,