Skip to content

[PM-35198] - remove orphaned blob attachments after deletion#7539

Open
jaasen-livefront wants to merge 2 commits intomainfrom
PM-35198
Open

[PM-35198] - remove orphaned blob attachments after deletion#7539
jaasen-livefront wants to merge 2 commits intomainfrom
PM-35198

Conversation

@jaasen-livefront
Copy link
Copy Markdown
Collaborator

@jaasen-livefront jaasen-livefront commented Apr 23, 2026

🎟️ Tracking

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

Blob cleanup ticket for reference

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

📔 Objective

Fix a type confusion bug in the POST /ciphers/attachment/validate/azure EventGrid handler
that caused orphaned attachment blobs in the attachments-v2 container to persist permanently
and never be cleaned up.

Root cause

The orphan-cleanup branch checked whether _attachmentStorageService is AzureSendFileStorageService
— the concrete type used by the Send file storage pipeline. The field is injected as
IAttachmentStorageService, which resolves to AzureAttachmentStorageService at runtime.
Because the types are unrelated, the is check always evaluated to false, the
DeleteBlobAsync call was never reached, and every blob that arrived without a matching
cipher attachment record was silently skipped and left in the container indefinitely.

Fix

  • Replace the incorrect concrete-type guard with a check against
    _attachmentStorageService.FileUploadType == FileUploadType.Azure, which correctly
    identifies the attachment storage back-end without a downcast.
  • Perform orphan deletion through the existing IAttachmentStorageService.DeleteAttachmentAsync
    interface method, passing a CipherAttachment.MetaData with ContainerName set to
    attachments-v2 (the EventGrid-enabled container) and the AttachmentId parsed from the
    blob name.
  • Remove the now-unused organizationId deconstruction variable from
    AzureAttachmentStorageService.IdentifiersFromBlobName.

Test coverage

Added AzureValidateFile_WhenCipherNotFound_DeletesOrphanedBlob — a unit test
that posts a realistic EventGrid Microsoft.Storage.BlobCreated payload through the controller
and asserts that DeleteAttachmentAsync is called with the correct cipherId, attachmentId,
and ContainerName when no matching cipher record exists in the repository.

📸 Screenshots

N/A — no UI changes.

@jaasen-livefront jaasen-livefront requested a review from a team as a code owner April 23, 2026 23:51
Copy link
Copy Markdown
Contributor

@nick-livefront nick-livefront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the detailed diagnosis! Does anything need to be done with attachments that are already orphaned? Do they need to be cleaned up across environments?

@jaasen-livefront
Copy link
Copy Markdown
Collaborator Author

I appreciate the detailed diagnosis! Does anything need to be done with attachments that are already orphaned? Do they need to be cleaned up across environments?

@nick-livefront I was thinking the same. I'll reach out to Matt.

@jaasen-livefront
Copy link
Copy Markdown
Collaborator Author

@nick-livefront I've created a separate ticket to handle cleanup and linked it in the description.

@sonarqubecloud
Copy link
Copy Markdown

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.59%. Comparing base (cdfe428) to head (b9dacd2).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7539      +/-   ##
==========================================
+ Coverage   59.54%   59.59%   +0.05%     
==========================================
  Files        2089     2089              
  Lines       92452    92457       +5     
  Branches     8218     8218              
==========================================
+ Hits        55050    55103      +53     
+ Misses      35458    35405      -53     
- Partials     1944     1949       +5     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants