[PM-18210] Cipher key encryption error handling #5611

andrebispo5 · 2025-07-29T13:41:25Z

🎟️ Tracking

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

📔 Objective

After the user navigates to their vault view

If there is a corrupt cipher:

replace cipher name with “[error: failed to decrypt]”
show dialog alert with share option

If user selects the corrupt cipher

reopen the simple dialog alert with share option for the selected cipher

📸 Screenshots

Screen.Recording.2025-07-29.at.14.28.54.mov

Screen.Recording.2025-07-29.at.22.55.59.mov

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

…ipher decryption failures.

…tion when clicking a failed cipher.

github-actions · 2025-07-29T13:47:41Z

Checkmarx One – Scan Summary & Details – f896f6bc-c8b6-4a5d-abed-43fe0571bb8f

Great job! No new security vulnerabilities introduced in this pull request

app/src/main/res/values/strings.xml

...rc/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt

...tlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt

# Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt

codecov · 2025-07-30T13:39:38Z

Codecov Report

❌ Patch coverage is 92.38754% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.91%. Comparing base (2fa9ea1) to head (2331fcd).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
.../vault/repository/util/VaultSdkCipherExtensions.kt	88.57%	3 Missing and 1 partial ⚠️
...t/bitwarden/ui/vault/feature/vault/VaultContent.kt	42.85%	2 Missing and 2 partials ⚠️
...bitwarden/ui/vault/feature/vault/VaultViewModel.kt	93.65%	3 Missing and 1 partial ⚠️
...ui/vault/feature/vault/util/VaultDataExtensions.kt	95.16%	3 Missing ⚠️
...ult/feature/itemlisting/VaultItemListingContent.kt	66.66%	1 Missing and 1 partial ⚠️
...t/feature/itemlisting/VaultItemListingViewModel.kt	90.90%	1 Missing and 1 partial ⚠️
...itemlisting/util/VaultItemListingDataExtensions.kt	95.65%	0 Missing and 2 partials ⚠️
...ault/feature/itemlisting/VaultItemListingScreen.kt	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5611      +/-   ##
==========================================
+ Coverage   83.86%   83.91%   +0.04%     
==========================================
  Files         696      696              
  Lines       52703    52960     +257     
  Branches     7216     7243      +27     
==========================================
+ Hits        44200    44439     +239     
- Misses       5975     5988      +13     
- Partials     2528     2533       +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.

# Conflicts: # app/src/main/res/values/strings.xml

…d screen

SaintPatrck · 2025-08-12T18:45:10Z

...tlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt

+        .filter { cipherListView ->
+            cipherListView.determineListingPredicate(itemListingType)
+        }
+        .applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds)
+        .toFilteredList(vaultFilterType)


🎨 Can we extract this into an extension function. Something like,

Suggested change

.filter { cipherListView ->

cipherListView.determineListingPredicate(itemListingType)

}

.applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds)

.toFilteredList(vaultFilterType)

.applyFilters(

itemListingType = itemListingType,

vaultFilterType = vaultFilterType,

restrictItemTypesPolicyOrgIds = restrictItemTypesPolicyOrgIds,

)

private fun List<CipherListView>.applyFilters( itemListingType: VaultItemListingState.ItemListingType.Vault, vaultFilterType: VaultFilterType, restrictItemTypesPolicyOrgIds: List<String>, ): List<CipherListView> = this .filter { it.determineListingPredicate(itemListingType) } .applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds) .toFilteredList(vaultFilterType) )

SaintPatrck

⛏️ Minor, but I think these variable names could be improved for readability. The function is getting pretty complex. I'd like to break it up with helper/extension functions at some point, but updating the var names will at least help in the interim.

SaintPatrck · 2025-08-12T19:15:22Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

@@ -44,15 +45,36 @@ fun VaultData.toViewState(
    vaultFilterType: VaultFilterType,
    restrictItemTypesPolicyOrgIds: List<String>?,
 ): VaultState.ViewState {
-
-    val filteredCipherViewListWithDeletedItems =
+    val filteredAllCipherViewListWithDeletedItems =


val allCipherViews

SaintPatrck · 2025-08-12T19:15:56Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

            .applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds ?: emptyList())
            .toFilteredList(vaultFilterType)

-    val filteredCipherViewList = filteredCipherViewListWithDeletedItems
+    val filteredCipherViewList = filteredAllCipherViewListWithDeletedItems


val activeCipherViews

SaintPatrck · 2025-08-12T19:16:53Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

+    val filteredCipherViewList = filteredAllCipherViewListWithDeletedItems
+        .filter { it.deletedDate == null }
+
+    val filteredSuccessCipherViewList = decryptCipherListResult


val activeDecryptedCipherViews

SaintPatrck · 2025-08-12T19:17:18Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

+        .applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds ?: emptyList())
+        .toFilteredList(vaultFilterType)
+
+    val filteredFailureCipherViewList = decryptCipherListResult


val activeUndecryptableCipherViews

SaintPatrck · 2025-08-12T19:19:34Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

+        .applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds ?: emptyList())
+        .toFilteredList(vaultFilterType)


Similar to my previous comment, it would be nice to extract this common filtering logic into an ext function.

SaintPatrck · 2025-08-12T19:38:08Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt

+                    .mapNotNull {
+                        it.id
+                    }


We need to declare the lambda arg here or in the enclosing update { } block.

Suggested change

.mapNotNull {

it.id

}

.mapNotNull { cipher -> cipher.id }

SaintPatrck · 2025-08-12T19:45:34Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt

@@ -233,6 +282,7 @@ private fun CipherListView.toVaultItemOrNull(
            extraIconList = toLabelIcons(),
            shouldShowMasterPasswordReprompt = hasMasterPassword &&
                reprompt == CipherRepromptType.PASSWORD,
+            hasDecryptionError = hasDecryptionError,


Can we use hasDecryptionError to control the overflow options that are assigned? That would make the item.hasDecryptionError checks in VaultContent unnecessary. Something like...

overflowOptions = toOverflowOptions(...).takeUnless { hasDecryptionError }

or if we don't want toOverflowOptions() to execute...

overflowOptions = if (hasDecryptionError) { emptyList() } else { toOverflowOptions() }

and in VaultContent we can revert

overflowOptions = if (favoriteItem.hasDecryptionError) { persistentListOf() } else { favoriteItem.overflowOptions.toImmutableList() },

SaintPatrck · 2025-08-12T19:48:03Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultContent.kt

+                    label = if (favoriteItem.hasDecryptionError) {
+                        stringResource(id = BitwardenString.error_cannot_decrypt)
+                    } else {
+                        favoriteItem.name()
+                    },


We can make this check unnecessary too, if we perform the hasDecryptionError check in VaultDataExtensions instead.

SaintPatrck · 2025-08-12T20:00:25Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingContent.kt

+                    label = if (isDecryptionErrorType) {
+                        stringResource(id = BitwardenString.error_cannot_decrypt)
+                    } else {
+                        it.title
+                    },


@andrebispo5 @david-livefront thoughts on making DisplayItem.title: Text instead of String?

That would allow us to assign title in VaultItemListingDataExtensions instead of having this if (decryptionError) check in the Composable.

No concern from me

SaintPatrck · 2025-08-12T20:02:05Z

...rc/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt

+    private fun handleShareCipherDecryptionErrorClick(cipherId: String?) {
+        sendEvent(
+            event = VaultItemListingEvent.ShowShareSheet(
+                content = cipherId.orEmpty(),


Instead of printing an empty string, can we print something that explicitly states the cipher does not have an ID? This should never happen, but if it does it will be easier to track down than sending an empty log.

andrebispo5 added 3 commits July 28, 2025 21:31

[PM-18210] Add new strings for feature

93747e7

[PM-18210] Update Vault and VaultItemListing data extensions to use c…

c5a5325

…ipher decryption failures.

[PM-18210] Add cipher failures to listings UI, add alert and share op…

8c25747

…tion when clicking a failed cipher.

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

...rc/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt Outdated Show resolved Hide resolved

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

...rc/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt Outdated Show resolved Hide resolved

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

...tlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt Outdated Show resolved Hide resolved

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

...tlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt Outdated Show resolved Hide resolved

SaintPatrck reviewed Jul 29, 2025

View reviewed changes

...tlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt Show resolved Hide resolved

andrebispo5 added 11 commits July 29, 2025 16:20

Merge branch 'main' into pm-18210/handle-cipher-decryption-errors

a6c3ab2

# Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt

[PM-18210] Change resource loading to BitwardenString

e3b8b64

[PM-18210] Remove unnecessary code

16e892d

[PM-18210] Update resources for plurals

cde8281

[PM-18210] Fix VaultItemListing decryption errors in folders

374123f

[PM-18210] Add decryption error cell to favorites

57d256a

[PM-18210] Add decryption error share sheet to vault handlers

608026c

[PM-18210] Update vault data extensions to handle decryption errors.

64fea90

[PM-18210] Add decryption error alerts and share sheets.

49b9188

[PM-18210] Code clean up

85712de

[PM-18210] Fix existing tests

f9b82dd

[PM-18210] Remove unnecessary suppresses

a3a87fa

andrebispo5 requested a review from SaintPatrck July 31, 2025 08:23

andrebispo5 changed the title ~~[Android] (WIP) Cipher key encryption error handling~~ [PM-18210] (WIP) Cipher key encryption error handling Jul 31, 2025

andrebispo5 changed the title ~~[PM-18210] (WIP) Cipher key encryption error handling~~ [PM-18210] Cipher key encryption error handling Jul 31, 2025

andrebispo5 added 4 commits July 31, 2025 10:01

[PM-18210] Move toFailureCipherListView to VauldSdkCipherExtension

68bbc36

[PM-18210] Remove test data.

3f995a3

[PM-18210] Add VaultSdkCipherExtensions tests

85edd0f

[PM-18210] Add failure cipher

488a02c

andrebispo5 added 3 commits July 31, 2025 12:42

Merge branch 'main' into pm-18210/handle-cipher-decryption-errors

43fa09d

# Conflicts: # app/src/main/res/values/strings.xml

[PM-18210] VaultViewModel tests

248537f

[PM-18210] VaultScreen tests

c52eb8b

pamperer562580892423 mentioned this pull request Aug 1, 2025

Attachment unavailable: created in 2024, now shows as "Old attachment that needs to be fixed", fix option is unavailable, download fails bitwarden/clients#15407

Closed

1 task

andrebispo5 added 3 commits August 4, 2025 14:52

[PM-18210] Decryption cipher failure: favourite and no folder test.

3487cd5

[PM-18210] Tests: decryption error to vault item listing viewmodel an…

9090c68

…d screen

[PM-18210] Increase vault item listing data test coverage

40c61da

andrebispo5 marked this pull request as ready for review August 5, 2025 11:36

andrebispo5 requested review from brian-livefront, david-livefront and a team as code owners August 5, 2025 11:36

[PM-18210] Increase code coverage.

2331fcd

SaintPatrck reviewed Aug 12, 2025

View reviewed changes

		.applyRestrictItemTypesPolicy(restrictItemTypesPolicyOrgIds ?: emptyList())
		.toFilteredList(vaultFilterType)

[PM-18210] Cipher key encryption error handling #5611

Are you sure you want to change the base?

[PM-18210] Cipher key encryption error handling #5611

Uh oh!

Conversation

andrebispo5 commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

github-actions bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SaintPatrck 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!

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!

SaintPatrck Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrebispo5 commented Jul 29, 2025 •

edited

Loading

github-actions bot commented Jul 29, 2025 •

edited

Loading

codecov bot commented Jul 30, 2025 •

edited

Loading

SaintPatrck Aug 12, 2025 •

edited

Loading