[PM-40277] Key rotation corrupts attachments, fido2 credentials

[mzieniukbw]

🎟️ Tracking

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

📔 Objective

SDK key rotation will currently corrupt fido2 credentials, attachments. This is because the cipherview has encrypted fields on it, namely fido2 credentials, attachment keys, which just get passed through.

Adds CipherView::generate_cipher_key_from, which takes the source key id explicitly (generate_cipher_key now delegates to it with User key id slot), and has the keyless cipher rotation path pass the real current key so the FIDO2 credentials and attachments are re-wrapped correctly.
Included test coverage that covers all cipher key rotation with fido2 and attachments possibilities.

🚨 Breaking Changes

None expected.
Android breaking change failure is unrelated to this PR.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed a focused bugfix for key rotation corrupting attachment keys and FIDO2 credentials on keyless legacy ciphers. The fix introduces CipherView::generate_cipher_key_from, which unwraps existing sub-keys under an explicitly supplied source_key instead of deriving it from key_identifier() — correcting the rotation path where sub-keys are wrapped under a Local-slot user key rather than the User/Organization slot. Traced the full re-encryption flow (decrypt_for_blob_upgrade → generate_cipher_key_from → encrypt_blob_cipher_with_wrapping_key) and confirmed the freshly installed per-item key prevents blob sealing from regenerating and clobbering the rewrapped sub-keys. Backward compatibility is preserved: the existing generate_cipher_key delegates with behaviorally identical arguments.

Code Review Details

No blocking findings.

Notes from analysis (no action required):

• The keyless path correctly relies on decrypt_cipher_key returning source_key when self.key is None, so sub-keys are unwrapped under current_key where they were actually wrapped.
• Keyless attachments (no key) are handled gracefully by the if let Some guard in AttachmentView::reencrypt_key.
• Test coverage is thorough: an 8-case rotation matrix (keyless/keyed × plain/fido2/attachment/both) plus two targeted unit tests for FIDO2 and attachment key rewrapping using distinct non-User slots.

[github-actions]

🔍 SDK Breaking Change Detection

SDK Version: km/pm-40277-key-rotation-corrupts-attachments-fido2-credentials (43a483a)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client	Status	Details
typescript	✅ No breaking changes detected	Compilation passed with new SDK version - View Details
android	✅ No breaking changes detected	Compilation passed with new SDK version - View Details

---

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

[quexten]

In my opinion, we should re-structure the vault item encryption. It still feels very error-prone and brittle, but that shouldn't be part of this work.

[codecov]

Codecov Report

❌ Patch coverage is 98.55596% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.90%. Comparing base (15ab4ca) to head (dc9cbe2).

Files with missing lines	Patch %	Lines
...den-vault/src/cipher/cipher_client/admin/create.rs	0.00%	1 Missing ⚠️
...arden-vault/src/cipher/cipher_client/admin/edit.rs	0.00%	1 Missing ⚠️
...bitwarden-vault/src/cipher/cipher_client/create.rs	0.00%	1 Missing ⚠️
...s/bitwarden-vault/src/cipher/cipher_client/edit.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1265      +/-   ##
==========================================
+ Coverage   85.86%   85.90%   +0.04%     
==========================================
  Files         493      493              
  Lines       70931    71145     +214     
==========================================
+ Hits        60906    61120     +214     
  Misses      10025    10025              

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