[PM-41472] feat: add bulk folder delete endpoint by gbubemismith · Pull Request #8157 · bitwarden/server

gbubemismith · 2026-08-06T17:52:23Z

🎟️ Tracking

📔 Objective

VFO1 introduces a new My Folders page in the web client, where users can multiselect folders and delete them in bulk. There is no bulk folder delete anywhere in the stack today, so the client has to issue one DELETE /folders/{id} per selected folder — N round trips for a single user action, and non-atomic.

This adds DELETE /folders, which takes a list of folder ids and deletes them in one request.

Add DELETE /folders for deleting multiple personal folders in one request, with a Folder_DeleteByIds stored procedure and matching EF implementation. Also fixes EF single-folder delete, which left ciphers pointing at the deleted folder and never bumped the account revision date.

mkincaid-bw · 2026-08-06T22:20:22Z

+            [UserId] = @UserId
+            AND [Status] = 2 -- Confirmed
+    )
+    UPDATE


Since there's no explicit transaction here and none in the C# caller, are there any concerns if these 3 data modification statements do not all complete atomically?

Nice catch. It wasn't atomic as written, although it was fail safe
Fixed 55067ab

github-actions · 2026-08-07T15:32:43Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the full PR diff against the merge base, including the merge of main into the branch at 0d8e0470c (that merge brings in unrelated PAM/AccessRule work and does not touch the folder bulk-delete code). The new DELETE /folders endpoint is feature-flagged with vfo1-foundation, derives the user id from the authenticated principal, and enforces ownership at three layers: the command filters requested ids against the caller's folders, the EF repository filters on UserId, and Folder_DeleteByIds builds an @OwnedIds table restricted to [UserId] = @UserId. The stored procedure mirrors the existing Folder_DeleteById cipher-unfiling logic (personal plus org ciphers reachable through collection/group access), the Dapper call wraps it in an explicit transaction, and the EF path performs the equivalent work plus UserBumpAccountRevisionDateAsync; the new DeleteAsync override brings the EF single-folder delete to parity with MSSQL.

Code Review Details

No findings at this revision.

Notes verified during review:

Migration 2026-08-12_00_AddFolderDeleteByIds.sql uses CREATE OR ALTER (idempotent) and is ordered after 2026-08-11_01 already on main; the SSDT file under src/Sql/dbo/ matches.
The endpoint follows the existing CiphersController.DeleteMany conventions ([HttpDelete("")], 500-item cloud cap, self-hosted exemption), so request-model handling is consistent with the established pattern.
The EF join over UserCipherDetailsQuery mirrors CipherRepository.MoveAsync; the modified Cipher entities come from a tracking query on dbContext.Ciphers, and no global QueryTrackingBehavior override exists, so the folder-map updates persist on SaveChangesAsync.
Previously raised threads (stored-procedure atomicity, over-broad EF cipher materialization, integration tests not asserting on the other user's folder / cipher unfiling) are addressed in the current code.

claude · 2026-08-07T15:37:58Z

+        var userCipherDetails = new UserCipherDetailsQuery(userId).Run(dbContext);
+        var filedCiphers = from ucd in userCipherDetails
+                           join c in dbContext.Ciphers.Where(c => c.Folders != null)
+                               on ucd.Id equals c.Id
+                           select c;
+
+        await filedCiphers.ForEachAsync(cipher =>


⚠️ IMPORTANT: This materializes and tracks every cipher the user can access, not just the ones filed under the deleted folders.

Details and fix

UserCipherDetailsQuery returns the user's personal ciphers plus every org cipher they can reach through a collection. The only server-side narrowing here is c.Folders != null, which is true for any cipher that any member has filed. So for a user in an org with 50k shared items, deleting a single folder streams and change-tracks ~50k Cipher rows (including the Data blob) into the DbContext. ForEachAsync streams, but tracked entities accumulate for the lifetime of the context, so peak memory scales with the accessible vault, not with the folders being deleted.

This also now applies to the pre-existing single-folder path, since DeleteAsync was overridden to delegate here.

A server-side filter on the user's key in the Folders map narrows this to only ciphers this user has filed, and translates to a LIKE on all three EF providers:

var userKey = userId.ToString(); var filedCiphers = from ucd in userCipherDetails join c in dbContext.Ciphers.Where(c => c.Folders != null && c.Folders.Contains(userKey)) on ucd.Id equals c.Id select c;

The same Folders.Contains(userId.ToString()) guard is already used in CipherRepository (src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs:617), so the JSON key format is consistent.

Addressed in 58fbb34

claude · 2026-08-07T15:37:59Z

+
+        await folderRepository.DeleteManyAsync([ownFolder.Id, otherUsersFolder.Id], user.Id);
+
+        Assert.Null(await folderRepository.GetByIdAsync(ownFolder.Id, user.Id));


⚠️ IMPORTANT: This test never asserts the property it is named for — the other user's folder is never checked.

Details and fix

otherUsersFolder and the other user's cipher are created and passed into DeleteManyAsync, but the only assertion is that ownFolder was deleted. The test passes today and would keep passing if the AND [UserId] = @UserId filter in Folder_DeleteByIds (or the f.UserId == userId predicate in the EF path) were dropped — which is exactly the cross-user data-deletion regression this test exists to catch.

Assert.Null(await folderRepository.GetByIdAsync(ownFolder.Id, user.Id)); Assert.NotNull(await folderRepository.GetByIdAsync(otherUsersFolder.Id, otherUser.Id));

Asserting the other user's cipher is still filed under otherUsersFolder.Id would also cover the JSON_MODIFY scoping.

claude · 2026-08-07T15:38:00Z

+
+        await folderRepository.DeleteManyAsync([deletedFolder.Id], user.Id);
+
+        Assert.Null(await folderRepository.GetByIdAsync(deletedFolder.Id, user.Id));


♻️ DEBT: Tests named for cipher unfiling create ciphers but never assert on them.

Details and fix

DeleteManyAsync_DeletesRequestedFolders_AndUnfilesTheirCiphers creates cipherInDeletedFolder, cipherInKeptFolder, unfiledCipher, and keptFolder, then asserts only that deletedFolder is gone. DeleteAsync_UnfilesTheCiphersInTheDeletedFolder (line 107) has the same shape.

The unfiling behavior is the newly added part of the EF path, and it is only covered by DeleteManyAsync_DeletesEveryRequestedFolder. Adding the assertions these tests already have the fixtures for closes the gap:

Assert.NotNull(await folderRepository.GetByIdAsync(keptFolder.Id, user.Id)); Assert.Null((await cipherRepository.GetByIdAsync(cipherInDeletedFolder.Id, user.Id)).FolderId); Assert.Equal(keptFolder.Id, (await cipherRepository.GetByIdAsync(cipherInKeptFolder.Id, user.Id)).FolderId);

codecov · 2026-08-07T15:41:11Z

Codecov Report

❌ Patch coverage is 86.02151% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.48%. Comparing base (643e3aa) to head (0d8e047).

Files with missing lines	Patch %	Lines
...tyFramework/Vault/Repositories/FolderRepository.cs	84.78%	5 Missing and 2 partials ⚠️
...ture.Dapper/Vault/Repositories/FolderRepository.cs	76.47%	4 Missing ⚠️
...rc/Core/Vault/Commands/DeleteManyFoldersCommand.cs	87.50%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8157      +/-   ##
==========================================
+ Coverage   68.41%   68.48%   +0.07%     
==========================================
  Files        2380     2381       +1     
  Lines      103679   103771      +92     
  Branches     9386     9394       +8     
==========================================
+ Hits        70931    71068     +137     
+ Misses      30420    30375      -45     
  Partials     2328     2328

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mkincaid-bw

LGTM

…ing in tests

nick-livefront

Only a single question but I'm assuming that no changes will come of it and I will learn something 😄

nick-livefront · 2026-08-10T15:11:22Z

+        // Deleting folders also re-assigns the ciphers filed under them, so clients need a full vault sync
+        // rather than a per-folder delete notification.


👏 Great comment

nick-livefront · 2026-08-10T15:13:43Z

+    [HttpDelete("")]
+    public async Task DeleteMany([FromBody] FolderBulkDeleteRequestModel model)
+    {
+        if (!_globalSettings.SelfHosted && model.Ids.Count() > 500)


❓ Why is selfhosted accounted for here?

We generally do this cause self-hosted is a single tenant the user owns and large operations will only affect them. For cloud instances we will want more control and a user submitting 10k+ will degrade for everyone

💯 Makes sense!

nick-livefront · 2026-08-10T15:16:43Z

+        var userCipherDetails = new UserCipherDetailsQuery(userId).Run(dbContext);
+        var filedCiphers = from ucd in userCipherDetails
+                           join c in dbContext.Ciphers.Where(c => c.Folders != null)
+                               on ucd.Id equals c.Id
+                           select c;
+
+        await filedCiphers.ForEachAsync(cipher =>


Addressed in 58fbb34

nick-livefront

@gbubemismith Looks like the database tests are still failing? Or didn't get re-run.

gbubemismith · 2026-08-12T18:17:42Z

@gbubemismith Looks like the database tests are still failing? Or didn't get re-run.

Yeah, doesn't look related to this PR

gbubemismith added ai-review-vnext Request a Claude code review using the vNext workflow t:feature Change Type - Feature Development labels Aug 6, 2026

Merge branch 'main' into vault/pm-41472/add-bulk-folder-delete-endpoint

35212a9

gbubemismith marked this pull request as ready for review August 6, 2026 18:43

gbubemismith requested review from a team as code owners August 6, 2026 18:43

gbubemismith requested a review from jaasen-livefront August 6, 2026 18:43

gbubemismith mentioned this pull request Aug 6, 2026

[PM-41472] feat(web): add the My folders page bitwarden/clients#22267

Draft

Merge branch 'main' into vault/pm-41472/add-bulk-folder-delete-endpoint

3ee0524

mkincaid-bw reviewed Aug 6, 2026

View reviewed changes

Merge branch 'main' into vault/pm-41472/add-bulk-folder-delete-endpoint

01c049f

claude Bot reviewed Aug 7, 2026

View reviewed changes

Wrap Folder_DeleteByIds in a transaction at the repository level

55067ab

gbubemismith requested a review from mkincaid-bw August 7, 2026 17:52

mkincaid-bw previously approved these changes Aug 7, 2026

View reviewed changes

Rename folder delete migration to resolve sequence collision

45a373d

gbubemismith dismissed mkincaid-bw’s stale review via 45a373d August 7, 2026 18:12

gbubemismith and others added 2 commits August 7, 2026 15:47

Narrow EF cipher scan, correct GetByIdAsync nullability, assert unfil…

26fcd5f

…ing in tests

Merge branch 'main' into vault/pm-41472/add-bulk-folder-delete-endpoint

58fbb34

nick-livefront previously approved these changes Aug 10, 2026

View reviewed changes

Feature gatted endpoint

d1c985c

gbubemismith dismissed nick-livefront’s stale review via d1c985c August 11, 2026 14:29

Renamed migration

fb37bb6

gbubemismith requested review from mkincaid-bw and nick-livefront August 12, 2026 17:21

nick-livefront approved these changes Aug 12, 2026

View reviewed changes

Merge branch 'main' into vault/pm-41472/add-bulk-folder-delete-endpoint

0d8e047

gbubemismith enabled auto-merge (squash) August 12, 2026 18:56

mkincaid-bw approved these changes Aug 12, 2026

View reviewed changes

gbubemismith merged commit e93b962 into main Aug 12, 2026
46 checks passed

gbubemismith deleted the vault/pm-41472/add-bulk-folder-delete-endpoint branch August 12, 2026 20:06


		await folderRepository.DeleteManyAsync([ownFolder.Id, otherUsersFolder.Id], user.Id);

		Assert.Null(await folderRepository.GetByIdAsync(ownFolder.Id, user.Id));

		// Deleting folders also re-assigns the ciphers filed under them, so clients need a full vault sync
		// rather than a per-folder delete notification.

Uh oh!

Conversation

gbubemismith commented Aug 6, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Bitwarden Claude Code Review

Uh oh!

claude Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

nick-livefront left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nick-livefront left a comment

Choose a reason for hiding this comment

Uh oh!

gbubemismith commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gbubemismith commented Aug 6, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading

codecov Bot commented Aug 7, 2026 •

edited

Loading