Skip to content

Remove ecdsa-keys compilation flag#1209

Open
neuronull wants to merge 1 commit into
mainfrom
dn/pm-38832/remove-ecdsa-keys-compilation-flag
Open

Remove ecdsa-keys compilation flag#1209
neuronull wants to merge 1 commit into
mainfrom
dn/pm-38832/remove-ecdsa-keys-compilation-flag

Conversation

@neuronull

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

We built with this flag as the default in #1181

, and now that the ssh agent in the clients repo supports the key type, there isn't a reason to have the compilation flag anymore, so this change is to remove it and cleanup.

@neuronull neuronull self-assigned this Jun 23, 2026
@neuronull neuronull requested a review from a team as a code owner June 23, 2026 19:03
@neuronull neuronull requested a review from dereknance June 23, 2026 19:03
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR removes the temporary ecdsa-keys compilation flag from bitwarden-ssh, making ECDSA key generation and import (P-256/P-384/P-521) unconditional now that the clients' SSH agent supports the key type. The change cleanly removes all #[cfg(feature = "ecdsa-keys")] gates, the reject_ecdsa_import guard and its single call site, and the obsolete import_ecdsa_blocked test, while promoting the previously gated ECDSA tests to run unconditionally. The feature flag is also dropped from the bitwarden-uniffi and bitwarden-wasm-internal dependency declarations.

Code Review Details

No issues found. The cleanup is complete and self-consistent:

  • A repository-wide search confirms zero remaining ecdsa-keys references.
  • The p256/p384/p521/ssh-key ECDSA dependencies were already unconditional, so removing the feature flag introduces no new build inputs or dependency changes.
  • ECDSA import/export/generation paths remain covered by tests, which now execute by default.

@sonarqubecloud

Copy link
Copy Markdown

@neuronull neuronull added the ai-review Request a Claude code review label Jun 23, 2026
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: dn/pm-38832/remove-ecdsa-keys-compilation-flag (6fdcf58)

⚠️ 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.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.09%. Comparing base (f20702c) to head (6fdcf58).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1209      +/-   ##
==========================================
- Coverage   85.09%   85.09%   -0.01%     
==========================================
  Files         464      464              
  Lines       63866    63861       -5     
==========================================
- Hits        54348    54343       -5     
  Misses       9518     9518              

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

@quexten quexten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does not need my explicit approval, but from @bitwarden/wg-ssh-keys side looks great.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants