Skip to content

Commit 6de1f2f

Browse files
committed
fix(account-deletion): durably purge a deleted workspace's storage by prefix
Account deletion found the objects to erase by reading their rows before the teardown transaction and deleting them in memory after commit. Anything that read could not see stayed in storage forever: rows written between the read and the workspace lock, rowless objects (table snapshots, compiled documents, table-import leftovers, execution objects whose metadata insert failed), everything past the 100k in-memory cap, everything after a crash following commit, and the account's private files in workspaces that survive it, whose rows cascade away with the user. Inside the deletion transaction, after the workspace rows are locked, account deletion now queues delayed outbox work that commits or rolls back with it: - one prefix sweep per deleted workspace across workspace/, table-import/, execution/, table-snapshots/, copilot-doc-compiled/ and workspace-logos/ in the bucket serving each, with a trailing-slash prefix built from a validated id; - exact-key cleanup for what cascades outside those prefixes: knowledge-base documents, Copilot uploads and other non-prefixed files of deleted workspaces, and the account's private files elsewhere (attachments of its own chats, private contexts, files with no workspace). Handed-off, Project, organization files and attachments of someone else's chat are not queued. Both wait out every upload URL signed before the deletion, refuse as a no-op when the workspace or account row exists again (restore, PITR), and keep any key a surviving workspace_files, workspace_file, workspace_file_version, document or execution_large_values row still references. The sweep reuses the Project prefix cleanup's provider listing and deletion, generalized by storage context and given an in-run provider cursor; Project cleanup still lists one page per run and deletes exactly as before. The existing in-memory purge is unchanged and still runs. Large execution values now pass cleanupOnMetadataFailure, so a failed metadata insert deletes the freshly keyed object instead of orphaning it, as execution files already do.
1 parent ae6ac39 commit 6de1f2f

9 files changed

Lines changed: 1349 additions & 24 deletions

File tree

‎apps/sim/lib/core/outbox/handlers.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ import {
4444
WORKSPACE_FILE_LIVE_DOC_OUTBOX_EVENT,
4545
WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT,
4646
} from '@/lib/uploads/contexts/workspace/file-outbox-events'
47+
import {
48+
ACCOUNT_STORAGE_OBJECT_CLEANUP_EVENT,
49+
ACCOUNT_WORKSPACE_STORAGE_SWEEP_EVENT,
50+
} from '@/lib/users/storage-sweep-events'
4751
import { WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS } from '@/lib/workflows/deployment-outbox-events'
4852
import { MIGRATED_INVITATION_EMAIL_EVENT_TYPE } from '@/lib/workspaces/admin-move-event'
4953
import {
@@ -231,6 +235,13 @@ export const OUTBOX_HANDLER_GROUPS: readonly LazyOutboxHandlerGroup[] = [
231235
return projectFilePrefixCleanupOutboxHandlers
232236
},
233237
},
238+
{
239+
events: [ACCOUNT_WORKSPACE_STORAGE_SWEEP_EVENT, ACCOUNT_STORAGE_OBJECT_CLEANUP_EVENT],
240+
load: async () => {
241+
const { accountStorageSweepOutboxHandlers } = await import('@/lib/users/storage-sweep')
242+
return accountStorageSweepOutboxHandlers
243+
},
244+
},
234245
{
235246
events: [PROJECT_FILE_DOCUMENT_RETIRE_EVENT],
236247
load: async () => {
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { mkdtemp, readdir, rm } from 'node:fs/promises'
2+
import { tmpdir } from 'node:os'
3+
import { join } from 'node:path'
4+
import {
5+
largeValueMetadataMock,
6+
largeValueMetadataMockFns,
7+
} from '@sim/testing/mocks/large-value-metadata.mock'
8+
import { uploadsConfigMock } from '@sim/testing/mocks/uploads-config.mock'
9+
import {
10+
uploadsMetadataMock,
11+
uploadsMetadataMockFns,
12+
} from '@sim/testing/mocks/uploads-metadata.mock'
13+
import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock'
14+
import { describeError } from '@sim/utils/errors'
15+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
16+
17+
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
18+
vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
19+
vi.mock('@/lib/uploads/server/metadata', () => uploadsMetadataMock)
20+
vi.mock('@/lib/execution/payloads/large-value-metadata', () => largeValueMetadataMock)
21+
22+
import { storeLargeValue } from '@/lib/execution/payloads/store'
23+
24+
const { mockInsertFileMetadata, mockDeleteFileMetadata } = uploadsMetadataMockFns
25+
const scope = { workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-1' }
26+
27+
let directory: string
28+
29+
async function storedObjects(): Promise<string[]> {
30+
try {
31+
return await readdir(join(directory, 'execution', ...Object.values(scope)))
32+
} catch (error) {
33+
if (describeError(error).code === 'ENOENT') return []
34+
throw error
35+
}
36+
}
37+
38+
describe('large value persistence against real local storage', () => {
39+
beforeEach(async () => {
40+
directory = await mkdtemp(join(tmpdir(), 'sim-large-value-'))
41+
setUploadDirServer(directory)
42+
mockInsertFileMetadata.mockReset().mockResolvedValue({ id: 'file-1' })
43+
mockDeleteFileMetadata.mockReset().mockResolvedValue(true)
44+
largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true)
45+
})
46+
47+
afterEach(async () => {
48+
await rm(directory, { recursive: true, force: true })
49+
})
50+
51+
it('removes the uploaded object when its metadata row cannot be written', async () => {
52+
mockInsertFileMetadata.mockRejectedValueOnce(new Error('workspace no longer exists'))
53+
const json = JSON.stringify({ payload: 'x'.repeat(2048) })
54+
55+
await expect(
56+
storeLargeValue(JSON.parse(json), json, Buffer.byteLength(json), {
57+
...scope,
58+
userId: 'user-1',
59+
requireDurable: true,
60+
})
61+
).rejects.toThrow('Failed to persist large execution value')
62+
63+
expect(await storedObjects()).toEqual([])
64+
expect(largeValueMetadataMockFns.mockRegisterLargeValueOwner).not.toHaveBeenCalled()
65+
})
66+
67+
it('keeps the object once its metadata row is written', async () => {
68+
const json = JSON.stringify({ payload: 'y'.repeat(2048) })
69+
70+
const ref = await storeLargeValue(JSON.parse(json), json, Buffer.byteLength(json), {
71+
...scope,
72+
userId: 'user-1',
73+
requireDurable: true,
74+
})
75+
76+
expect((await storedObjects()).filter((name) => name.endsWith('.json'))).toEqual([
77+
ref.key?.split('/').pop(),
78+
])
79+
})
80+
})

‎apps/sim/lib/execution/payloads/store.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ async function persistValue(
9090
context: 'execution',
9191
preserveKey: true,
9292
customKey: key,
93+
cleanupOnMetadataFailure: true,
9394
metadata: {
9495
originalName: `large-value-${id}.json`,
9596
uploadedAt: new Date().toISOString(),

‎apps/sim/lib/projects/files/prefix-cleanup.ts‎

Lines changed: 93 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from '@/lib/projects/files/outbox-events'
2020
import {
2121
getStorageConfig,
22+
type StorageContext,
2223
USE_BLOB_STORAGE,
2324
USE_GCS_STORAGE,
2425
USE_S3_STORAGE,
@@ -45,6 +46,11 @@ interface CleanupPayload {
4546

4647
/** Expired signatures stop new transfers, but a transfer already in progress may finish later. */
4748
export function projectUploadCleanupAvailableAt(now = new Date()): Date {
49+
return storageCleanupAvailableAt(now)
50+
}
51+
52+
/** Earliest moment no upload URL signed before `now` can still write an object. */
53+
export function storageCleanupAvailableAt(now = new Date()): Date {
4854
return new Date(now.getTime() + UPLOAD_URL_TTL_MS + CLOCK_SKEW_MS)
4955
}
5056

@@ -242,57 +248,113 @@ async function localKeys(prefix: string, signal: AbortSignal): Promise<string[]>
242248
return keys
243249
}
244250

245-
async function listKeys(prefix: string, signal: AbortSignal): Promise<string[]> {
251+
/** One provider page of object keys; `done` once the listing has nothing after this page. */
252+
interface StorageKeyPage {
253+
keys: string[]
254+
done: boolean
255+
/** Provider cursor for the following page, valid only within the current run. */
256+
nextPageToken?: string
257+
}
258+
259+
/**
260+
* Lists one page of object keys under `prefix` in the bucket serving `context`. A local listing
261+
* has no cursor: it returns the first keys still on disk, so callers advance by deleting them.
262+
*/
263+
export async function listStorageKeyPage(
264+
prefix: string,
265+
context: StorageContext,
266+
signal: AbortSignal,
267+
pageToken?: string
268+
): Promise<StorageKeyPage> {
246269
signal.throwIfAborted()
247270
if (USE_S3_STORAGE) {
248271
const { ListObjectsV2Command } = await import('@aws-sdk/client-s3')
249272
const { getS3Client } = await import('@/lib/uploads/providers/s3/client')
250-
const config = getStorageConfig('project')
273+
const config = getStorageConfig(context)
251274
const page = await getS3Client().send(
252-
new ListObjectsV2Command({ Bucket: config.bucket, Prefix: prefix, MaxKeys: PAGE_SIZE }),
275+
new ListObjectsV2Command({
276+
Bucket: config.bucket,
277+
Prefix: prefix,
278+
MaxKeys: PAGE_SIZE,
279+
...(pageToken ? { ContinuationToken: pageToken } : {}),
280+
}),
253281
{ abortSignal: signal }
254282
)
255-
return (page.Contents ?? []).flatMap((object) => (object.Key ? [object.Key] : []))
283+
const nextPageToken = page.IsTruncated ? page.NextContinuationToken : undefined
284+
return {
285+
keys: (page.Contents ?? []).flatMap((object) => (object.Key ? [object.Key] : [])),
286+
done: !nextPageToken,
287+
...(nextPageToken ? { nextPageToken } : {}),
288+
}
256289
}
257290
if (USE_BLOB_STORAGE) {
258291
const { getBlobServiceClient } = await import('@/lib/uploads/providers/blob/client')
259-
const config = getStorageConfig('project')
260-
if (!config.containerName) throw new Error('Project blob container missing')
292+
const config = getStorageConfig(context)
293+
if (!config.containerName) throw new Error(`Blob container missing for ${context} storage`)
261294
const pages = (await getBlobServiceClient())
262295
.getContainerClient(config.containerName)
263296
.listBlobsFlat({ prefix, abortSignal: signal })
264-
.byPage({ maxPageSize: PAGE_SIZE })
297+
.byPage({ maxPageSize: PAGE_SIZE, ...(pageToken ? { continuationToken: pageToken } : {}) })
265298
const page = await pages.next()
266-
return page.done ? [] : page.value.segment.blobItems.map((object) => object.name)
299+
if (page.done) return { keys: [], done: true }
300+
const nextPageToken = page.value.continuationToken || undefined
301+
return {
302+
keys: page.value.segment.blobItems.map((object) => object.name),
303+
done: !nextPageToken,
304+
...(nextPageToken ? { nextPageToken } : {}),
305+
}
267306
}
268307
if (USE_GCS_STORAGE) {
269308
const { getGcsClient } = await import('@/lib/uploads/providers/google-cloud-storage/client')
270-
const config = getStorageConfig('project')
271-
if (!config.bucket) throw new Error('Project bucket missing')
272-
const [files] = await (await getGcsClient())
273-
.bucket(config.bucket)
274-
.getFiles({ prefix, maxResults: PAGE_SIZE, autoPaginate: false })
309+
const config = getStorageConfig(context)
310+
if (!config.bucket) throw new Error(`Bucket missing for ${context} storage`)
311+
const [files, nextQuery] = await (await getGcsClient()).bucket(config.bucket).getFiles({
312+
prefix,
313+
maxResults: PAGE_SIZE,
314+
autoPaginate: false,
315+
...(pageToken ? { pageToken } : {}),
316+
})
275317
signal.throwIfAborted()
276-
return files.map((file) => file.name)
318+
const nextPageToken =
319+
isRecordLike(nextQuery) && typeof nextQuery.pageToken === 'string' && nextQuery.pageToken
320+
? nextQuery.pageToken
321+
: undefined
322+
return {
323+
keys: files.map((file) => file.name),
324+
done: !nextPageToken,
325+
...(nextPageToken ? { nextPageToken } : {}),
326+
}
277327
}
278-
return localKeys(prefix, signal)
328+
const keys = await localKeys(prefix, signal)
329+
return { keys, done: keys.length < PAGE_SIZE }
279330
}
280331

281-
async function sweepPrefix(projectId: string, context: OutboxEventContext): Promise<boolean> {
282-
const prefix = prefixFor(projectId)
283-
const keys = await listKeys(prefix, context.signal)
332+
/**
333+
* Deletes listed keys, each of which must sit under `prefix`, and prunes local directories the
334+
* deletions empty, never above the prefix itself. Stops before another deletion once the handler
335+
* deadline is near; `complete` is false when it did.
336+
*/
337+
export async function deleteStorageKeysUnderPrefix(
338+
prefix: string,
339+
context: StorageContext,
340+
keys: readonly string[],
341+
outbox: Pick<OutboxEventContext, 'signal' | 'deadlineAt'>
342+
): Promise<{ deleted: number; complete: boolean }> {
343+
if (prefix.length < 2 || !prefix.endsWith('/')) throw new Error('Unbounded storage prefix')
284344
let deleted = 0
285345
for (const key of keys) {
286-
if (!key.startsWith(prefix)) throw new Error('Storage provider returned a foreign Project key')
287-
context.signal.throwIfAborted()
288-
if (deleted > 0 && context.deadlineAt && Date.now() + 5000 >= context.deadlineAt) return false
346+
if (!key.startsWith(prefix))
347+
throw new Error('Storage provider returned a key outside the prefix')
348+
outbox.signal.throwIfAborted()
349+
if (deleted > 0 && outbox.deadlineAt && Date.now() + 5000 >= outbox.deadlineAt)
350+
return { deleted, complete: false }
289351
try {
290-
await deleteFile({ key, context: 'project', signal: context.signal })
352+
await deleteFile({ key, context, signal: outbox.signal })
291353
} catch (error) {
292354
if (describeError(error).code !== 'ENOENT') throw error
293355
}
294356
if (!USE_S3_STORAGE && !USE_BLOB_STORAGE && !USE_GCS_STORAGE) {
295-
const root = join(UPLOAD_DIR_SERVER, 'project', projectId)
357+
const root = join(UPLOAD_DIR_SERVER, prefix.slice(0, -1))
296358
let directory = dirname(join(UPLOAD_DIR_SERVER, key))
297359
while (directory === root || directory.startsWith(`${root}/`)) {
298360
await removeEmptyDirectory(directory)
@@ -301,7 +363,14 @@ async function sweepPrefix(projectId: string, context: OutboxEventContext): Prom
301363
}
302364
deleted++
303365
}
304-
return keys.length < PAGE_SIZE
366+
return { deleted, complete: true }
367+
}
368+
369+
async function sweepPrefix(projectId: string, context: OutboxEventContext): Promise<boolean> {
370+
const prefix = prefixFor(projectId)
371+
const { keys } = await listStorageKeyPage(prefix, 'project', context.signal)
372+
const { complete } = await deleteStorageKeysUnderPrefix(prefix, 'project', keys, context)
373+
return complete && keys.length < PAGE_SIZE
305374
}
306375

307376
const cleanupPrefix: OutboxHandler<unknown> = async (raw, context) => {

‎apps/sim/lib/users/account-deletion.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
} from '@/lib/table/rows/executions'
5555
import type { StorageContext } from '@/lib/uploads'
5656
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
57+
import { enqueueAccountStorageCleanupInTx } from '@/lib/users/storage-sweep'
5758
import {
5859
listSharedResourceWorkspaceIdsForUser,
5960
listUnresolvedSharedResourceWorkspaces,
@@ -861,6 +862,11 @@ export async function deleteUserAccount(userId: string): Promise<AccountDeletion
861862
* workspace and user deletes cascade their rows away; delivered after commit.
862863
*/
863864
stoppedRuns = await stopLiveRunsTx(tx, { userId })
865+
/**
866+
* Queued while the doomed workspaces are locked and their rows still exist: every object
867+
* that cascades away below gets a durable, delayed cleanup that commits with the deletion.
868+
*/
869+
await enqueueAccountStorageCleanupInTx(tx, { userId, workspaceIds: doomedWorkspaceIds })
864870
if (doomedWorkspaceIds.length > 0) {
865871
/**
866872
* Re-checked here rather than trusted from the plan: a workspace that
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export const ACCOUNT_WORKSPACE_STORAGE_SWEEP_EVENT = 'account-deletion.storage.workspace-sweep'
2+
export const ACCOUNT_STORAGE_OBJECT_CLEANUP_EVENT = 'account-deletion.storage.object-cleanup'

0 commit comments

Comments
 (0)