Skip to content

[PM-40505] Detecting old attachments in diagnostics tool - #22580

Open
mzieniukbw wants to merge 2 commits into
mainfrom
km/pm-40505-old-attachment-detection-diagnostic-tool
Open

[PM-40505] Detecting old attachments in diagnostics tool#22580
mzieniukbw wants to merge 2 commits into
mainfrom
km/pm-40505-old-attachment-detection-diagnostic-tool

Conversation

@mzieniukbw

@mzieniukbw mzieniukbw commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

Show and logs ciphers that have old attachments as a problem in the diagnostics tool. Each cipher is clickable, which routes the user to vault to that specific cipher, where they have 'Fix' button to re-upload the attachment.

Old attachments lacks key and it's content is encrypted by user key.

📸 Screenshots

Old attachment:

Screen.Recording.2026-08-21.at.16.49.50.mov
image logs: image

No old attachments:
image
logs:
image

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.35%. Comparing base (91a576b) to head (86c12b9).
⚠️ Report is 22 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #22580      +/-   ##
==========================================
+ Coverage   54.15%   54.35%   +0.19%     
==========================================
  Files        4312     4320       +8     
  Lines      137026   137429     +403     
  Branches    21686    21753      +67     
==========================================
+ Hits        74208    74697     +489     
+ Misses      57399    57303      -96     
- Partials     5419     5429      +10     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mzieniukbw mzieniukbw added the t:feature Change Type - Feature Development label Aug 21, 2026
@mzieniukbw
mzieniukbw marked this pull request as ready for review August 21, 2026 15:50
@mzieniukbw
mzieniukbw requested a review from a team as a code owner August 21, 2026 15:50
@mzieniukbw
mzieniukbw requested a review from quexten August 21, 2026 15:50
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new AttachmentStep diagnostics step, the RecoveryStep/StepState additions that carry per-step detail into the view, the template rendering of the message and vault links, and the new en locale keys. Verified the /vault?itemId=…&action=edit target resolves through VaultComponent's query-param handling to editCipherId, that RouterLink and bitLink reach the standalone component via SharedModule, and that the $COUNT$/count placeholder pair substitutes correctly in TranslationService. The organization-cipher filter matches CipherStep's documented scope, and the key == null check is equivalent to the existing AttachmentView.isLegacyAttachment() rule. No security or correctness defects found.

Code Review Details
  • ♻️ : Re-implements the canonical "old attachment" predicate rather than sharing one definition with CipherView.hasOldAttachments
    • apps/web/src/app/key-management/data-recovery/steps/attachment-step.ts:28
  • ♻️ : Generic RecoveryStep contract carries oldAttachmentCipherIds, a field named for one concrete step
    • apps/web/src/app/key-management/data-recovery/steps/recovery-step.ts:21

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

Labels

t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant