fix(backup): trace an account made for an upload when it is made

The upload creates a passwordless account before it reads the file and
deletes it again when the file is refused. The only audit row was
written after a successful store, so a refused upload whose delete also
failed left an account nobody could sign into and no record of where it
came from.

The account and its admin.user.create row (outcome "created") now
commit together, and the discard writes admin.user.delete with outcome
"deleted" or "failed", so a leftover account always has a trace. The
runbook says how to remove one by hand.

docs/ops/backup-restore.md

@@ -103,7 +103,12 @@ exists, and the console then asks where the records should go:
   password or email, as an ordinary account with the user role, and nobody
   can sign into it yet. Before handing it over, set its first password with
   **Reset password** in Admin → Users; the person can change it, add an email
-  and enrol a passkey from their settings.
+  and enrol a passkey from their settings. The account is made before the
+  file is read, with an `admin.user.create` audit row (`outcome: "created"`),
+  and removed again when the file is refused, with an `admin.user.delete` row
+  (`reason: "backup_upload_refused"`). When that row says `outcome: "failed"`,
+  the removal did not happen and the account is left over, holding nothing:
+  delete it in Admin → Users.
 - **An existing account**, for someone who has signed up again. Pick the
   account and type its username to confirm. The account must hold no records
   yet: a restore replaces an account's records and does not merge them, so an

src/__tests__/baselines/source-function-lines.json

@@ -1,10 +1,10 @@
 {
   "$comment": "Production functions over 200 lines, keyed \"file :: name\", and their length. Maintained by src/__tests__/source-size-ratchet.test.ts.",
   "count": 345,
-  "total": 111940,
+  "total": 111929,
   "entries": {
     "app/achievements/page.tsx :: AchievementsPage": 206,
-    "app/api/admin/backups/upload/route.ts :: apiHandler(arg)": 286,
+    "app/api/admin/backups/upload/route.ts :: apiHandler(arg)": 275,
     "app/api/admin/notifications/reminder-check/route.ts :: apiHandler(arg)": 206,
     "app/api/admin/settings/route.ts :: apiHandler(arg) #2": 211,
     "app/api/auth/login/route.ts :: apiHandler(arg)": 262,

src/app/api/admin/backups/upload/route.ts

@@ -295,17 +295,42 @@ async function checkTargetBeforeRead(
   return null;
 }
 
+/** Who made an account for a copy, for the trail. */
+interface UploadActor {
+  adminId: string;
+  ipAddress: string | null;
+}
+
 /**
  * The account made for a copy. Created before the file is read, because the
  * file is rewritten to its id on the way in, and deleted again when nothing
  * is stored. No password, no email: nobody can sign into it until an admin
- * sets its first password (Admin, Users, Reset password).
+ * sets its first password (Admin, Users, Reset password). Its audit row
+ * commits with it, so an account this route made always has a trace, even
+ * when the upload fails and removing it again fails too.
  */
-async function createTargetAccount(username: string): Promise<ChosenTarget> {
+async function createTargetAccount(
+  username: string,
+  actor: UploadActor,
+): Promise<ChosenTarget> {
   try {
-    const account = await prisma.user.create({
-      data: { username, role: "USER" },
-      select: { id: true, username: true },
+    const account = await prisma.$transaction(async (tx) => {
+      const row = await tx.user.create({
+        data: { username, role: "USER" },
+        select: { id: true, username: true },
+      });
+      await auditLog("admin.user.create", {
+        userId: actor.adminId,
+        ipAddress: actor.ipAddress,
+        client: tx,
+        details: {
+          targetUserId: row.id,
+          username: row.username,
+          reason: "backup_restore",
+          outcome: "created",
+        },
+      });
+      return row;
     });
     return { ...account, created: true };
   } catch (err) {
@@ -355,13 +380,20 @@ async function ownerOfRetargetedFile(
 /**
  * Delete the account an upload created for its copy, when nothing was stored.
  * It holds nothing and nobody can have signed into it (no password, no email),
- * so the row is all there is. A failure is noted, never thrown over the
+ * so the row is all there is. Either way the trail says what became of it: a
+ * removal that failed leaves an account to delete by hand
+ * (`docs/ops/backup-restore.md`). A failure is noted, never thrown over the
  * refusal the operator needs to see.
  */
-async function discardCreatedAccount(userId: string): Promise<void> {
+async function discardCreatedAccount(
+  account: ChosenTarget,
+  actor: UploadActor,
+): Promise<void> {
+  let outcome: "deleted" | "failed" = "deleted";
   try {
-    await prisma.user.delete({ where: { id: userId } });
+    await prisma.user.delete({ where: { id: account.id } });
   } catch (err) {
+    outcome = "failed";
     annotate({
       meta: {
         backup_upload_account_discard_failed: true,
@@ -370,6 +402,20 @@ async function discardCreatedAccount(userId: string): Promise<void> {
       },
     });
   }
+  try {
+    await auditLog("admin.user.delete", {
+      userId: actor.adminId,
+      ipAddress: actor.ipAddress,
+      details: {
+        targetUserId: account.id,
+        username: account.username,
+        reason: "backup_upload_refused",
+        outcome,
+      },
+    });
+  } catch {
+    annotate({ meta: { backup_upload_account_discard_unaudited: true } });
+  }
 }
 
 export const POST = apiHandler(async (request: NextRequest) => {
@@ -379,6 +425,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
   await assertRecentCookieProof(admin, session.id);
   annotate({ action: { name: "admin.backups.upload" } });
   const ipAddress = getClientIp(request);
+  const actor: UploadActor = { adminId: admin.id, ipAddress };
 
   // Rate-limit identical to the manual-run endpoint. Three is plenty for a
   // deliberate restore-prep workflow.
@@ -494,7 +541,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
   let rewrite: BackupOwnerRewrite | null = null;
   if (choice?.into === "new") {
     try {
-      target = await createTargetAccount(choice.username);
+      target = await createTargetAccount(choice.username, actor);
     } catch (err) {
       if (err instanceof UploadRefused) return refuse(err);
       throw err;
@@ -583,7 +630,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
     );
   } catch (err) {
     // Nothing was stored, so the account made for the copy goes again.
-    if (chosen?.created) await discardCreatedAccount(chosen.id);
+    if (chosen?.created) await discardCreatedAccount(chosen, actor);
     if (err instanceof UploadRefused) return refuse(err);
     // The file was fine; its encrypted copy is larger than the stored-copy
     // limit (`BACKUP_MAX_STORED_MB`). That is the operator's to act on, with
@@ -609,18 +656,6 @@ export const POST = apiHandler(async (request: NextRequest) => {
   };
   const storedOwner = owner as { id: string; username: string } | null;
 
-  if (chosen?.created) {
-    await auditLog("admin.user.create", {
-      userId: admin.id,
-      ipAddress,
-      details: {
-        targetUserId: chosen.id,
-        username: chosen.username,
-        reason: "backup_restore",
-        backupId: created.id,
-      },
-    });
-  }
   await auditLog("admin.backups.upload", {
     userId: admin.id,
     ipAddress,

tests/integration/backup-erased-account-restore.test.ts

@@ -677,17 +677,20 @@ describe("restoring the backup of an erased account into a chosen account", () =
         passwordHash: null,
       });
 
+      // Traced when it is made, before the file is read; the upload row
+      // links it to the copy.
       expect(await auditDetails(prisma, "admin.user.create")).toEqual([
         {
           targetUserId: account.id,
           username: "returning-person",
           reason: "backup_restore",
-          backupId,
+          outcome: "created",
         },
       ]);
       expect(await auditDetails(prisma, "admin.backups.upload")).toEqual([
-        expect.objectContaining({ ownerId: account.id, into: "new" }),
+        expect.objectContaining({ backupId, ownerId: account.id, into: "new" }),
       ]);
+      expect(await auditDetails(prisma, "admin.user.delete")).toEqual([]);
       expect(await cellsNaming(prisma, OWNER_ID)).toEqual([]);
 
       // How the person gets in: the admin sets a first password with the
@@ -861,7 +864,24 @@ describe("restoring the backup of an erased account into a chosen account", () =
       expect(
         await prisma.user.count({ where: { username: "would-be-copy" } }),
       ).toBe(0);
-      expect(await auditDetails(prisma, "admin.user.create")).toEqual([]);
+      // The account was made, traced, and removed again, traced as well.
+      const made = await auditDetails(prisma, "admin.user.create");
+      expect(made).toEqual([
+        {
+          targetUserId: expect.any(String),
+          username: "would-be-copy",
+          reason: "backup_restore",
+          outcome: "created",
+        },
+      ]);
+      expect(await auditDetails(prisma, "admin.user.delete")).toEqual([
+        {
+          targetUserId: (made[0] as { targetUserId: string }).targetUserId,
+          username: "would-be-copy",
+          reason: "backup_upload_refused",
+          outcome: "deleted",
+        },
+      ]);
       expect(await prisma.dataBackup.count()).toBe(0);
 
       // Without a target, the same file goes to its own account as before.
@@ -879,6 +899,50 @@ describe("restoring the backup of an erased account into a chosen account", () =
     SLOW,
   );
 
+  it(
+    "leaves a trace of an account made for a refused file when it cannot be removed again",
+    async () => {
+      const prisma = getPrismaClient();
+      const { json } = await eraseAfterExport(prisma);
+      await createOwner(prisma);
+      const { prisma: appPrisma } = await import("@/lib/db");
+      const remove = vi
+        .spyOn(appPrisma.user, "delete")
+        .mockRejectedValueOnce(new Error("connection reset"));
+
+      const refused = await uploadInto(json, {
+        into: "new",
+        username: "left-behind",
+      });
+      remove.mockRestore();
+      // The operator still sees why the file was refused.
+      expect(refused.status, JSON.stringify(refused.body)).toBe(409);
+      expect(refused.body.meta?.errorCode).toBe("backup.owner.exists");
+
+      const account = await prisma.user.findUniqueOrThrow({
+        where: { username: "left-behind" },
+      });
+      expect(account).toMatchObject({ passwordHash: null, email: null });
+      expect(await auditDetails(prisma, "admin.user.create")).toEqual([
+        {
+          targetUserId: account.id,
+          username: "left-behind",
+          reason: "backup_restore",
+          outcome: "created",
+        },
+      ]);
+      expect(await auditDetails(prisma, "admin.user.delete")).toEqual([
+        {
+          targetUserId: account.id,
+          username: "left-behind",
+          reason: "backup_upload_refused",
+          outcome: "failed",
+        },
+      ]);
+    },
+    SLOW,
+  );
+
   it(
     "refuses a non-admin session and an admin's Bearer token, and creates nothing",
     async () => {