fix(account-deletion): durably purge a deleted account's storage without touching live objects by waleedlatif1 · Pull Request #9146 · simstudioai/sim

waleedlatif1 · 2026-10-16T22:41:30Z

Summary

Account deletion collects storage keys before its teardown transaction and purges them in memory after commit. Objects could survive an erasure request indefinitely, even though the privacy policy promises Account Data is removed within 30 days of a deletion request. The ways an object escaped:

Window: rows written by still-running executions between collection and the workspace lock cascaded away uncollected.
No row: table snapshots, compiled documents and table-import leftovers have no rows to collect.
Failed metadata insert: a large-value object whose metadata insert failed was left behind.
Cap: keys past the 100k cap were never purged.
Crash: a crash between commit and the in-memory purge lost every key.
Surviving workspaces: the deleted user's own private files there lost their rows to the cascade.

Durable, delayed cleanup enqueued inside the deletion transaction (after the existing workspace FOR UPDATE; no new locks):

Per deleted workspace, a prefix sweep over workspace/, table-import/, execution/, table-snapshots/, copilot-doc-compiled/ and workspace-logos/, each followed by <id>/.
- It generalizes lib/projects/files/prefix-cleanup.ts; Project behavior is unchanged.
- It runs after the upload-URL TTL plus skew, resumes per prefix, and refuses as a no-op if the workspace row exists again.
Exact-key batches for non-prefixable keys: KB document.storage_key, copilot/<uuid>/, and the user's private files and versions whose rows cascade with the user.
- A workspace-less row is queued only for context copilot.
- Profile-picture and org-logo keys are never queued; the profile picture still comes only from user.image.

Safety guards: a key is kept, never deleted, if any of these hold:

it is outside a validated <id>/ prefix (ids are validated; prefixes always end in /);
it sits under a workspace root that still exists;
a surviving row references it: workspace_files.key, workspace_file.key, workspace_file_version.key, document.storage_key or execution_large_values.key;
for publicly served prefixes only, a stored asset URL names it: user.image, workspace.logo_url, custom_block.icon_url, chat.customizations, oauth_client.icon, or organization.logo / whitelabel_settings / metadata.
- The URL read is narrowed by a key-derived substring and post-filtered.
- It tolerates malformed URLs.

Further rules:

Every guard failure keeps everything (fail-closed).
A local sidecar is kept with its object.
References embedded in free text or JSON (workflow sub-blocks, chat messages, Markdown) are not consulted. For the deleted user's own chat attachments that is intended erasure.

Also:

The existing in-memory purge now goes through the same reference guard; it previously deleted shared legacy kb/ keys.
Large-value uploads pass cleanupOnMetadataFailure.
Prefix sweeps count only real removals. A full page that removes nothing defers and spends an attempt, which fixes an infinite no-attempt loop, including the same pre-existing flaw in Project cleanup.

Deliberately left behind (never live-data loss): legacy objects under profile-pictures/, organization-logos/ or og-images/ that workspace_files rows in a deleted workspace name are no longer deleted, either in the deletion request or by the durable cleanup. Whitelabel logos were once uploaded as profile-picture rows, so such an object may still be served publicly through a surviving organization's settings; these objects are left orphaned rather than risk deleting live data. Logos under the deleted workspace's own workspace-logos/<id>/ prefix are still swept. The account's own avatar (the key its user.image names) is still deleted, unless another account or a stored asset URL still names it.

Type of Change

Bug fix

Testing

Unit tests on real local storage cover:
- prefix boundary (ws1 vs ws10);
- restore refusal (workspace and account);
- row and URL references;
- malformed URLs;
- the gate on non-public keys;
- resumable paging;
- sidecars;
- stall accounting;
- payload validation.
Each guard was reverted and its test went red.
lib/users/storage-sweep.integration.ts (real Postgres) runs the real deleteUserAccount:
- the delay is honored;
- an object uploaded during the delay is purged;
- rowless and private families are purged;
- a neighbouring-id prefix, a cross-workspace referenced key, a handed-off file, another user's chat attachment and a URL-referenced logo all survive;
- a restored workspace is a no-op.
Three independent adversarial reviews (data-loss hunt, regression/DB-load, URL-guard soundness) found no remaining live-data deletion path.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced

… 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.

Review of the durable storage sweep found ways it, or the in-memory purge it backs up, could delete an object another tenant still uses. Reference guard. Every account-deletion purge now runs one check before deleting a key: the durable prefix sweep (per listed key), the durable exact-key cleanup, and the post-commit in-memory purge. A key is kept when a row still names it (workspace_files.key, workspace_file.key, workspace_file_version.key, document.storage_key, execution_large_values.key) or a stored asset URL names it: user.image, workspace.logo_url, organization.logo, organization.whitelabel_settings, organization.metadata, custom_block.icon_url, chat.customizations (deployed-chat logo and avatar) and oauth_client.icon. URLs are mapped to keys with extractStorageKey, so serve paths with or without a provider segment and direct bucket URLs both match; over-matching only keeps an object. References embedded in free-form JSON or text (workflow sub-block values, chat messages, Markdown links) are not consulted; for the deleted account's own chat attachments that is the intended erasure. This keeps whitelabel logos that were uploaded under an admin's personal workspace prefix, and legacy kb/ objects shared across workspaces, which the in-memory purge used to delete unchecked. Exact-key cleanup also keeps any key under a workspace-keyed root whose workspace still exists, since its members can read it by key through the metadata fallback. Workspace-less rows are queued only for the Copilot context: the workspace binding check was added NOT VALID, so legacy shared-context rows without a workspace can name a surviving workspace's object. Profile pictures are taken only from the account's own user.image, never from rows, because whitelabel logos were once stored as profile-pictures rows under an admin's account; organization logos are never queued either. Prefix deletion now counts only objects actually removed. A tolerated ENOENT (locally, any file whose on-disk name no sanitized key resolves to) used to count as progress, so a page of such files re-listed forever without spending an attempt. The workspace sweep and the Project prefix cleanup (which had the same flaw) now defer and spend an attempt when a full page removes nothing. A local metadata sidecar is kept whenever its object is. Versions of the account's private files are queued in keyset pages that stop at the cap instead of one unbounded query inside the deletion transaction.

A stored URL with malformed percent-encoding (for example `/api/files/serve/%E0%A4%A`, which any workspace admin can store as a logo URL) made extractStorageKey throw. The guard then rejected on every call, so every storage purge on the platform failed closed and deleted nothing. The guard now parses each value under its own try and falls back to the raw, undecoded path. extractStorageKey itself is unchanged for its other callers. The URL-column check read six unindexed tables on almost every guard call. Only keys under the prefixes the file-serve route answers without a row or a session (profile-pictures/, og-images/, workspace-logos/, organization-logos/, per isPublicByKeyPrefix in the serve route) can be live through a stored URL alone. The guard now reads URL columns only for such keys, and only rows containing the key's longest URL-safe run, which percent-encoding never splits. Each run is read at most once per guard, 50 per query. A key with no run long enough falls back to the unfiltered read. Lookup failures still throw, so nothing is deleted. The workspace sweep now defers with a distinct reason when a full local page removes nothing, so the test for it can tell a stall from a deadline.

vercel · 2026-10-16T22:41:35Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 17, 2026 12:00am UTC

cubic-dev-ai

All reported issues were addressed across 11 files

_{This PR changes concurrency-sensitive code such as locks, queues, or retries. Ultrareviews find 2.4x more serious bugs than standard reviews. Comment @cubic-dev-ai ultrareview to run one.

Turn on auto-fix | Re-trigger cubic}

greptile-apps · 2026-10-16T22:47:09Z

Confidence Score: 5/5

[High impact] The latest changes appear safe to merge; no new blocking issue was found.

Summary

Adds delayed, durable storage cleanup to account deletion while keeping objects that surviving rows, workspaces, or saved asset URLs still use.

Account deletion queues storage cleanup that keeps objects still in use.
Project cleanup uses the shared paged storage sweeper.
Failed metadata writes remove the large-value upload.
File serving passes request cancellations through its error handling.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Collect exact storage keys] --> B[Lock account and workspace rows]
  B --> C[Read newly written account files]
  C --> D[Queue delayed cleanup and delete account]
  D --> E[Commit]
  E --> F[Outbox runs after upload URL expiry]
  F --> G{Account or workspace restored?}
  G -->|Yes| H[Keep objects]
  G -->|No| I[Check surviving references]
  I --> J[Keep referenced objects]
  I --> K[Delete unreferenced objects]

_{Reviews (5) · Last reviewed commit: "fix(account-deletion): queue the account..." · Reviewed by Greptile}

…re storage primitives - keysNamedByUrl also yields a leniently decoded candidate when a stored URL's percent-encoding is malformed, so a legacy key holding a literal '%' is kept - correct the guard's TSDoc: rowless keys stay reachable through key paths under a live workspace and object metadata, not only through a row - move the reference guard to lib/users/storage-reference-guard.ts - move prefix listing/deletion, the upload-TTL cleanup delay, and the outbox deadline margin from lib/projects/files/prefix-cleanup.ts to lib/uploads/core/prefix-sweep.ts; drop the projectUploadCleanupAvailableAt alias - share WORKSPACE_SHARED_FILE_CONTEXTS between member handoff and account cleanup - isPublicStorageKey replaces the public-prefix lists in the serve route and guard

…locks - collectAccountStorageObjects runs the exact-key keyset reads before the transaction, beside collectAccountStorageKeys; inside it, enqueueAccountStorageCleanupInTx now only inserts the prefix-sweep events and the pre-collected exact-key events, so teammates' writes in shared workspaces never wait on those reads - Project prefix cleanup completes on a short listing page again, as before - the file-serve route keeps its original public-prefix check

waleedlatif1 · 2026-10-16T22:59:50Z

@greptile

waleedlatif1 · 2026-10-16T22:59:51Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-16T22:59:56Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 19 files

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

- integration fixture rows get unique names: the active-name unique index on workspace_files rejected the second 'fixture.txt' row, failing seed() - the reference guard reads stored asset URLs unfiltered and decodes them, so a URL that percent-encodes the key's characters still keeps its object; the world-readable-key gate still decides whether any URL column is read - enqueueAccountStorageCleanupInTx re-reads the account's rows written since the pre-transaction collection (via the user_id and uploaded_by indexes) under the account lock, so a late Copilot or knowledge-base upload is queued - local listings walk keys in sorted order and page after the last key, so a workspace sweep advances past a full page of kept objects

waleedlatif1 · 2026-10-16T23:20:06Z

@greptile

waleedlatif1 · 2026-10-16T23:20:07Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-16T23:20:12Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

…tely

cubic-dev-ai

1 issue found and verified against the latest diff

Confidence score: 2/5

In sweepWorkspaceStorage, the one-time workspace existence check leaves a race: if the workspace is restored during the sweep, later deletions can remove restored or newly uploaded objects. Recheck the workspace state before deleting objects so the sweep is safe against a concurrent restore.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/lib/users/storage-sweep.ts">

<violation number="1" location="apps/sim/lib/users/storage-sweep.ts:277">
P1: `sweepWorkspaceStorage` checks whether the workspace exists only once, before sweeping all families and pages. If the workspace is restored after that read, the handler can delete its restored and newly uploaded objects; re-check the workspace immediately before each destructive page/batch or serialize the sweep with restoration.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

…s per page - fix TS2769: the collection scope passed a readonly id list to notInArray - the reference guard reads stored asset URL rows only where they mention a public key's owning id run, paged, and keeps a public key with no such run instead of reading every URL column - the account deletion request no longer purges world-readable keys; the account's own avatar is queued for the delayed durable cleanup instead - the workspace prefix sweep re-checks that the workspace is still gone before every page it deletes from

waleedlatif1 · 2026-10-16T23:46:11Z

@greptile

waleedlatif1 · 2026-10-16T23:46:12Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-16T23:46:17Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 20 files

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

user.image accepts any serve URL, so it names a key without proving whose it is. The avatar is queued for durable cleanup only when the account's completed profile_picture upload session, or an older profile-picture workspace_files row of the account, names the same key; otherwise it is left in place.

- the reference guard keeps keys under a live workspace's keyed roots itself, so the request purge gets the check the durable cleanup already had - stored asset URLs are re-read for every guard call rather than cached for the sweep, so a reference written mid-sweep still keeps its object - an avatar key that is not a safe profile-pictures key is never queued

…eep 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.

waleedlatif1 · 2026-10-17T00:01:02Z

@greptile

waleedlatif1 · 2026-10-17T00:01:03Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-17T00:01:10Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 21 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic}

waleedlatif1 added 3 commits October 16, 2026 14:55

vercel Bot temporarily deployed to Preview October 16, 2026 22:41 Inactive

cubic-dev-ai Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/projects/files/prefix-cleanup.ts Outdated

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

Comment thread apps/sim/lib/users/storage-sweep.ts Outdated

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/storage-sweep.ts Outdated

Comment thread apps/sim/lib/users/storage-sweep.ts Outdated

vercel Bot temporarily deployed to Preview October 16, 2026 22:50 Inactive

vercel Bot temporarily deployed to Preview October 16, 2026 22:54 Inactive

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/account-deletion.ts

cubic-dev-ai Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/account-deletion.ts

Comment thread apps/sim/lib/users/account-deletion.ts

vercel Bot temporarily deployed to Preview October 16, 2026 23:18 Inactive

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/storage-reference-guard.ts Outdated

docs(account-deletion): describe the in-transaction delta read accura…

c3e5e65

…tely

vercel Bot temporarily deployed to Preview October 16, 2026 23:27 Inactive

This was referenced Oct 16, 2026

chore: red proof A for #9146 — do not merge #9152

Closed

chore: red proof B for #9146 — do not merge #9153

Closed

cubic-dev-ai Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/storage-sweep.ts Outdated

Comment thread apps/sim/lib/users/storage-reference-guard.ts Outdated

vercel Bot temporarily deployed to Preview October 16, 2026 23:45 Inactive

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/storage-sweep.ts Outdated

cubic-dev-ai Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread apps/sim/lib/users/account-deletion.ts

Comment thread apps/sim/lib/users/storage-reference-guard.ts Outdated

vercel Bot temporarily deployed to Preview October 16, 2026 23:54 Inactive

vercel Bot temporarily deployed to Preview October 16, 2026 23:57 Inactive

vercel Bot temporarily deployed to Preview October 17, 2026 00:00 Inactive

cubic-dev-ai Bot reviewed Oct 17, 2026

View reviewed changes

waleedlatif1 merged commit 678c1a4 into staging Oct 17, 2026
54 checks passed

waleedlatif1 deleted the fix/account-deletion-storage-sweep branch October 17, 2026 00:25

Conversation

waleedlatif1 commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Diagram

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 16, 2026

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 16, 2026

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

waleedlatif1 commented Oct 16, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 16, 2026

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Oct 17, 2026

Uh oh!

waleedlatif1 commented Oct 17, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 17, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

waleedlatif1 commented Oct 16, 2026 •

edited

Loading

vercel Bot commented Oct 16, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

greptile-apps Bot commented Oct 16, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading