fix(account-deletion): queue the account's avatar and let the guard keep shared ones

An ownership lookup left avatars uploaded more than a week before deletion in
storage, since their upload sessions are gone by then. The key user.image
names is queued again, and the run-time reference guard keeps it whenever
another account's user.image or a stored asset URL still names it.

Author: waleedlatif1

apps/sim/lib/users/storage-reference-guard.ts

@@ -207,11 +207,12 @@ async function loadKeysNamedByUrls(runs: readonly string[], named: Set<string>):
  * - it sits under a world-readable root (`isPublicStorageKey`, the same prefixes the file-serve
  *   route's `isPublicByKeyPrefix` answers without a session) and a stored asset URL names it.
  *   Only URL rows that mention the key's {@link ownerRun} are read, on every call so a reference
- *   written mid-sweep still counts; a public key without one is kept unread. A URL that hand-encodes characters of that
- *   run is missed. Account deletion asks about public keys only under a deleted workspace's
- *   prefix (its logos) and for an avatar the account is proven to have uploaded, so such a URL can
- *   cost an object of a deleted workspace or of the erased account's own uploads, never one a
- *   surviving workspace, organization, or user uploaded.
+ *   written mid-sweep still counts; a public key without one is kept unread. A URL that
+ *   hand-encodes characters of that run is missed. Account deletion asks about public keys only
+ *   under a deleted workspace's prefix (its logos) and for the avatar the erased account's
+ *   `user.image` names, so such a URL can cost one of those, and only when no surviving reference
+ *   names it the way this app writes URLs; another user's own `user.image` naming their avatar
+ *   always keeps it.
  *
  * No URL column is read for any other key: `verifyFileAccess` serves it only through a row, by its
  * key path under a workspace that still exists (kept above), or by object metadata to a signed-in holder of a hand-pasted URL to

apps/sim/lib/users/storage-sweep.integration.ts

@@ -7,7 +7,6 @@ import {
   copilotChats,
   outboxEvent,
   permissions,
-  uploadSession,
   user,
   workspace,
   workspaceFiles,
@@ -104,26 +103,6 @@ async function insertAdminWorkspace(id: string, ownerId: string) {
   })
 }
 
-/** The ownership record a finished profile-picture upload leaves behind. */
-function completedAvatarUpload(userId: string, key: string) {
-  return {
-    id: generateId(),
-    tokenHash: generateId(),
-    userId,
-    purpose: 'profile_picture' as const,
-    method: 'put' as const,
-    storageContext: 'profile-pictures',
-    finalKey: key,
-    storageProvider: 'local' as const,
-    fileName: 'avatar.png',
-    contentType: 'image/png',
-    fileSize: 1,
-    status: 'completed' as const,
-    expiresAt: new Date(),
-    completedAt: new Date(),
-  }
-}
-
 function avatarUrl(key: string): string {
   return `/api/files/serve/${encodeURIComponent(key)}?context=profile-pictures`
 }
@@ -251,12 +230,6 @@ async function seed() {
       logoUrl: `/api/files/serve/s3/${encodeURIComponent(keys.logoNamedByEncodedUrl)}`,
     })
     .where(eq(workspace.id, survivingId))
-  await db
-    .insert(uploadSession)
-    .values([
-      completedAvatarUpload(departingId, keys.ownAvatar),
-      completedAvatarUpload(ownerId, keys.othersAvatar),
-    ])
   await db
     .update(user)
     .set({ image: avatarUrl(keys.ownAvatar) })
@@ -359,20 +332,15 @@ describe('account deletion storage sweep', () => {
     }
   )
 
-  check('never purges an avatar the account points at but did not upload', async () => {
+  check("keeps another account's avatar the departing image points at", async () => {
     const f = await seed()
     await db
       .update(user)
       .set({ image: avatarUrl(f.keys.othersAvatar) })
-      .where(eq(user.id, f.departingId))
+      .where(inArray(user.id, [f.ownerId, f.departingId]))
     await deleteUserAccount(f.departingId)
 
-    const objects = await objectEvents(f.departingId)
-    const queuedKeys = objects.flatMap((event) =>
-      (event.payload as { objects: { key: string }[] }).objects.map((object) => object.key)
-    )
-    expect(queuedKeys).not.toContain(f.keys.othersAvatar)
-    await runNow(objects.map((event) => event.id))
+    await runNow((await objectEvents(f.departingId)).map((event) => event.id))
 
     expect(await readFile(join(storageRoot, f.keys.othersAvatar), 'utf8')).toBe('x')
   })
@@ -403,7 +371,6 @@ afterAll(async () => {
         )
       )
     await db.delete(workspaceFiles).where(inArray(workspaceFiles.userId, userIds))
-    await db.delete(uploadSession).where(inArray(uploadSession.userId, userIds))
     await deleteWorkspaceFixture(db, inArray(workspace.id, workspaceIds))
     await db.delete(user).where(inArray(user.id, userIds))
     await rm(storageRoot, { recursive: true, force: true })

apps/sim/lib/users/storage-sweep.test.ts

@@ -361,6 +361,19 @@ describe('account storage object cleanup', () => {
       expect(dbChainMockFns.from).not.toHaveBeenCalledWith(schemaMock[table])
   })
 
+  it("keeps an avatar another account's image still names", async () => {
+    const key = 'profile-pictures/upload-0002-owner-avatar.png'
+    await put(key)
+    queueTableRows(schemaMock.user, [])
+    queueTableRows(schemaMock.user, [
+      { id: 'owner-1', value: `/api/files/serve/s3/${encodeURIComponent(key)}` },
+    ])
+
+    await cleanup(payload([{ key, context: 'profile-pictures' }]), outboxContext())
+
+    expect(await exists(key)).toBe(true)
+  })
+
   it("deletes the account's own uploaded avatar", async () => {
     await put('profile-pictures/upload-0001-avatar.png')
 
@@ -505,28 +518,16 @@ describe('queueing account storage cleanup', () => {
     queueTableRows(schemaMock.user, [
       { image: '/api/files/serve/s3/profile-pictures%2Fupload-0001-avatar.png' },
     ])
-    queueTableRows(schemaMock.uploadSession, [{ id: 'upload-0001' }])
 
     await queueCleanup()
 
     expect(queuedObjectKeys()).toEqual(['profile-pictures/upload-0001-avatar.png'])
   })
 
-  it('never queues an avatar key the account did not upload', async () => {
-    queueTableRows(schemaMock.user, [
-      { image: '/api/files/serve/s3/profile-pictures%2Fupload-0002-someone-else.png' },
-    ])
-
-    await queueCleanup()
-
-    expect(queuedObjectKeys()).toEqual([])
-  })
-
   it('never queues an avatar key that could escape its root', async () => {
     queueTableRows(schemaMock.user, [
       { image: '/api/files/serve/profile-pictures%2Fx%2F..%2F..%2Fworkspace%2Fother.png' },
     ])
-    queueTableRows(schemaMock.uploadSession, [{ id: 'upload-0003' }])
 
     await queueCleanup()
 

apps/sim/lib/users/storage-sweep.ts

@@ -3,7 +3,6 @@ import {
   copilotChats,
   document,
   knowledgeBase,
-  uploadSession,
   user,
   workspaceFile,
   workspaceFiles,
@@ -80,9 +79,8 @@ const OBJECT_CONTEXTS: ReadonlySet<string> = new Set<StorageContext>([
 /**
  * Keys no row collected here may name: Project prefix cleanup, the Assistant upload-session
  * sweep, organization logos, and profile pictures — which account deletion takes only from the
- * account's own `user.image`, and only once {@link uploadedProfilePicture} proves the account
- * uploaded it, because whitelabel logos were once uploaded as `profile-pictures` rows under an
- * admin's account.
+ * account's own `user.image` (see {@link extractProfilePictureKey}), because whitelabel logos were
+ * once uploaded as `profile-pictures` rows under an admin's account.
  */
 const FOREIGN_LIFECYCLE_ROOTS = [
   'project/',
@@ -143,10 +141,12 @@ function isAccountProfilePicture(key: string, context: string): boolean {
 }
 
 /**
- * The account's own uploaded avatar, as a storage key. `user.image` holds either a
- * `/api/files/serve/...` path (an upload this app owns) or an absolute URL from an OAuth provider,
- * which must not be deleted; only the former yields a key, and only under `profile-pictures/`.
- * The image is personal data, so an erasure that leaves it in the bucket is not an erasure.
+ * The account's avatar, as a storage key. `user.image` holds either a `/api/files/serve/...` path
+ * (an upload this app owns) or an absolute URL from an OAuth provider, which must not be deleted;
+ * only the former yields a key, and only under `profile-pictures/`. The image is personal data, so
+ * an erasure that leaves it in the bucket is not an erasure. `user.image` is user-settable, so the
+ * key can name someone else's avatar; the cleanup's reference guard keeps any key another
+ * account's `user.image` or a stored asset URL still names.
  */
 export function extractProfilePictureKey(image: string | null): string | null {
   if (!image) return null
@@ -471,44 +471,6 @@ async function collectVersionsOf(
   )
 }
 
-/**
- * Whether the account itself uploaded `key`. `user.image` is settable to any serve URL, so it
- * names a key without proving whose it is; the proof is the account's completed `profile_picture`
- * upload session (kept for a week after completion), or the `workspace_files` row an older
- * profile-picture upload wrote. Without either the avatar is left in place.
- */
-async function uploadedProfilePicture(
-  executor: DbOrTx,
-  userId: string,
-  key: string
-): Promise<boolean> {
-  const [session] = await executor
-    .select({ id: uploadSession.id })
-    .from(uploadSession)
-    .where(
-      and(
-        eq(uploadSession.finalKey, key),
-        eq(uploadSession.userId, userId),
-        eq(uploadSession.purpose, 'profile_picture'),
-        eq(uploadSession.status, 'completed')
-      )
-    )
-    .limit(1)
-  if (session) return true
-  const [row] = await executor
-    .select({ id: workspaceFiles.id })
-    .from(workspaceFiles)
-    .where(
-      and(
-        eq(workspaceFiles.key, key),
-        eq(workspaceFiles.userId, userId),
-        eq(workspaceFiles.context, 'profile-pictures')
-      )
-    )
-    .limit(1)
-  return Boolean(row)
-}
-
 interface CollectionScope {
   userId: string
   workspaceIds: string[]
@@ -534,8 +496,7 @@ async function collectInto(
     .where(eq(user.id, userId))
     .limit(1)
   const profilePicture = extractProfilePictureKey(account?.image ?? null)
-  if (profilePicture && (await uploadedProfilePicture(executor, userId, profilePicture)))
-    collector.addProfilePicture(profilePicture)
+  if (profilePicture) collector.addProfilePicture(profilePicture)
 
   const filesSince = since
     ? and(eq(workspaceFiles.userId, userId), gte(workspaceFiles.uploadedAt, since))