Skip to content

PM-26059: Remove CipherKeyEncryption feature flag#6617

Merged
david-livefront merged 1 commit intomainfrom
PM-26059-remove-cipher-key-encryption-flag
Mar 9, 2026
Merged

PM-26059: Remove CipherKeyEncryption feature flag#6617
david-livefront merged 1 commit intomainfrom
PM-26059-remove-cipher-key-encryption-flag

Conversation

@david-livefront
Copy link
Collaborator

@david-livefront david-livefront commented Mar 5, 2026

🎟️ Tracking

PM-26059

📔 Objective

This PR removes the CipherKeyEncryption feature flag from the app.

Note that there is still a server version check for this feature to ensure older servers that do not support the feature do not receive updated ciphers.

@david-livefront david-livefront requested a review from a team as a code owner March 5, 2026 21:35
@david-livefront david-livefront added app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt ai-review Request a Claude code review labels Mar 5, 2026
@github-actions github-actions bot added app:authenticator Bitwarden Authenticator app context and removed t:tech-debt Change Type - Tech debt labels Mar 5, 2026
@david-livefront david-livefront added the t:tech-debt Change Type - Tech debt label Mar 5, 2026
@claude
Copy link
Contributor

claude bot commented Mar 5, 2026

Claude finished @david-livefront's task in 3m 10s —— View job


Code Review: Remove CipherKeyEncryption feature flag

  • Gather context and review PR diff
  • Read changed files for full context
  • Post review findings

Verdict: This PR looks good. Clean feature flag removal with all references properly cleaned up.

Changes Summary

File Change
FlagKey.kt Removed CipherKeyEncryption data object and its entry from activePasswordManagerFlags
FeatureFlagManagerImpl.kt Replaced getCipherKeyEncryptionFlagState() (server version check AND feature flag) with isCipherKeyEncryptionEnabled extension property (server version check only)
FeatureFlagManagerTest.kt Updated expected value from false to true to match new behavior
DebugMenuViewModelTest.kt Swapped FlagKey.CipherKeyEncryption for FlagKey.DummyBoolean
FeatureFlagListItems.kt Removed CipherKeyEncryption from both ListItemContent and getDisplayLabel
strings_non_localized.xml Removed cipher_key_encryption string resource

Review Notes

Correctness: The refactoring in FeatureFlagManagerImpl.kt is correct. The old logic was:

isServerVersionAtLeast(...) && getFeatureFlag(FlagKey.CipherKeyEncryption)

The new logic is:

isServerVersionAtLeast(serverConfigStateFlow.value, CIPHER_KEY_ENC_MIN_SERVER_VERSION)

This properly preserves the server version gate while removing the feature flag dependency, which aligns with the PR description stating the server version check should be retained.

Completeness: Grep confirms no remaining references to CipherKeyEncryption, cipher-key-encryption, or cipher_key_encryption outside the expected constants in FeatureFlagManagerImpl.kt (which still passes "enableCipherKeyEncryption" to the SDK — this is correct and necessary).

Design: Extracting isCipherKeyEncryptionEnabled as a private extension property on ServerConfigRepository is a nice touch — it keeps the logic close to its only consumer while keeping the class body clean.

Test coverage: The test update (falsetrue) correctly reflects the new behavior. Codecov confirms all modified lines are covered.

CI: Lint passed, tests running. No new security vulnerabilities detected by Checkmarx.

No issues found.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Logo
Checkmarx One – Scan Summary & Detailsa3644a0d-0c24-4fa0-8204-42289128e8ad

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

@david-livefront david-livefront force-pushed the PM-26059-remove-cipher-key-encryption-flag branch from 4f46217 to ba70d5e Compare March 5, 2026 21:43
@github-actions github-actions bot removed the t:tech-debt Change Type - Tech debt label Mar 5, 2026
@david-livefront david-livefront added the t:tech-debt Change Type - Tech debt label Mar 5, 2026
@david-livefront david-livefront force-pushed the PM-26059-remove-cipher-key-encryption-flag branch from ba70d5e to 5c31a80 Compare March 5, 2026 21:44
@github-actions github-actions bot removed the t:tech-debt Change Type - Tech debt label Mar 5, 2026
@david-livefront david-livefront added the t:tech-debt Change Type - Tech debt label Mar 5, 2026
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.96%. Comparing base (aa23d5e) to head (0290986).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6617      +/-   ##
==========================================
+ Coverage   85.90%   85.96%   +0.06%     
==========================================
  Files         804      801       -3     
  Lines       57370    57325      -45     
  Branches     8307     8306       -1     
==========================================
- Hits        49284    49281       -3     
+ Misses       5203     5162      -41     
+ Partials     2883     2882       -1     
Flag Coverage Δ
app-data 17.53% <100.00%> (-0.01%) ⬇️
app-ui-auth-tools 20.88% <0.00%> (+<0.01%) ⬆️
app-ui-platform 15.07% <0.00%> (+<0.01%) ⬆️
app-ui-vault 25.79% <0.00%> (+<0.01%) ⬆️
authenticator 6.66% <0.00%> (-0.02%) ⬇️
lib-core-network-bridge 4.28% <0.00%> (+<0.01%) ⬆️
lib-data-ui 0.93% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@david-livefront david-livefront force-pushed the PM-26059-remove-cipher-key-encryption-flag branch from 5c31a80 to 0290986 Compare March 6, 2026 15:06
@github-actions github-actions bot removed the t:tech-debt Change Type - Tech debt label Mar 6, 2026
@david-livefront david-livefront added the t:tech-debt Change Type - Tech debt label Mar 6, 2026
@david-livefront
Copy link
Collaborator Author

Thanks @SaintPatrck

@david-livefront david-livefront added this pull request to the merge queue Mar 9, 2026
Merged via the queue into main with commit d88de04 Mar 9, 2026
27 checks passed
@david-livefront david-livefront deleted the PM-26059-remove-cipher-key-encryption-flag branch March 9, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants