[PM-33500] - delete attachments from deleted ciphers

[jaasen-livefront]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-33500

📔 Objective

Fix orphaned attachment files left behind when ciphers are deleted in bulk, an organization vault is purged, or an organization is deleted.

When a single cipher is deleted via CipherService.DeleteAsync, attachment files are properly removed from blob/disk storage via DeleteAttachmentsForCipherAsync. However, three other deletion paths only removed DB records, leaving attachment files permanently orphaned in storage:

1. CipherService.DeleteManyAsync (bulk cipher delete) — deleted cipher rows but never cleaned up attachment storage.
2. CipherService.PurgeAsync (org vault purge) — deleted cipher rows but never cleaned up attachment storage.
3. OrganizationDeleteCommand.DeleteAsync (org deletion) — cascade-deleted cipher rows via SQL stored proc but never cleaned up attachment storage.

Changes

• CipherService.DeleteManyAsync: After DB deletion, iterate deleted ciphers and call DeleteAttachmentsForCipherAsync for each.
• CipherService.PurgeAsync: Fetch org cipher IDs before DB deletion, then clean up each cipher's attachments after.
• OrganizationDeleteCommand.DeleteAsync: Inject ICipherRepository and IAttachmentStorageService. Fetch org cipher IDs before DB cascade delete, then clean up each cipher's attachments after.
• Updated unit tests for DeleteManyAsync (org admin + personal owner paths) and OrganizationDeleteCommand.Delete_Success to assert attachment cleanup. Added new PurgeAsync_WithOrganizationId_DeletesCiphersAndAttachments test.

📸 Screenshots

[github-actions]

Checkmarx One – Scan Summary & Details – 68c8fbd9-2cb3-4ed2-9823-b6a28f79b125

---

New Issues (2)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1592	details Method at line 1592 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1419	details Method at line 1419 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

---

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 293

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.69%. Comparing base (753c8c8) to head (3048893).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7208      +/-   ##
==========================================
+ Coverage   57.68%   57.69%   +0.01%     
==========================================
  Files        2035     2035              
  Lines       89645    89663      +18     
  Branches     7993     7996       +3     
==========================================
+ Hits        51709    51732      +23     
+ Misses      36072    36066       -6     
- Partials     1864     1865       +1     

☔ View full report in Codecov by Sentry.
📢 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.

[claude]

Claude finished @gbubemismith's task in 1m 46s —— View job

---

Code Review Complete

• Gather context and read changed files
• Review code changes for correctness, security, and best practices
• Post review findings

Result: Clean — No Issues Found

This PR correctly fixes orphaned attachment files left behind when ciphers are deleted in bulk, an organization vault is purged, or an organization is deleted. The implementation is consistent with the existing pattern in CipherService.DeleteAsync.

What was reviewed:

• OrganizationDeleteCommand.cs — Injects ICipherRepository and IAttachmentStorageService, fetches org cipher IDs before the cascade DB delete, then cleans up each cipher's attachment storage after.
• CipherService.cs — Adds attachment cleanup to DeleteManyAsync (iterates deleted ciphers) and PurgeAsync (fetches org cipher IDs before DB deletion, cleans up after).
• Test files — Unit tests properly assert that DeleteAttachmentsForCipherAsync is called for each cipher with attachments.

Key observations:

• The fetch-before-delete / cleanup-after pattern is the correct approach since DB records are needed to identify which ciphers have attachments.
• DeleteAttachmentsForCipherAsync implementations (Azure, Local, Noop) all safely handle cases where no attachments exist, so iterating all ciphers is safe.
• The approach is consistent with the existing single-cipher DeleteAsync path.

No bugs, security issues, or style concerns were identified.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud