Skip to content

Conversation

@trmartin4
Copy link
Member

@trmartin4 trmartin4 commented Dec 30, 2025

🎟️ Tracking

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

📔 Objective

Added Cargo.toml checks to dep-ownership.ts script, to match bitwarden/sdk-internal#639 for sdk-internal.

This required adding ownership for the following dependencies in order to get the lint to pass:

  • aes-gcm, chacha20poly1305 → Added to Key Management team
  • ashpd, ctor, secmem-proc, thiserror, zeroizing-alloc → Added to Platform team

⏰ 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

@trmartin4 trmartin4 changed the title Added Cardo dep ownership. Include Cargo dependencies in dep-ownership lint check Dec 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2025

Logo
Checkmarx One – Scan Summary & Details62c3c532-b677-45b3-9b5e-cfc88a482e95

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

@trmartin4 trmartin4 marked this pull request as ready for review December 30, 2025 18:22
@trmartin4 trmartin4 requested review from a team, Thomas-Avery and djsmith85 and removed request for a team December 30, 2025 18:23
@trmartin4
Copy link
Member Author

Tagging Platform and KM for review as I've made the judgement call on who should own these previously unowned Cargo dependencies.

@quexten
Copy link
Contributor

quexten commented Dec 30, 2025

@trmartin4 / @Thomas-Avery aes-gcm is only used by the tools team for the chromium importer feature, in order to decrypt chromium imports. KM does not use it in any capacity or offer any functionality on top of it, despite it being a crypto dependency, and there is no good way for KM to maintain it / test functionality if updates come through. For instance, if the aes-gcm crate has a breaking change, would KM be expected to update tools code to update the dep?

While KM does build the cryptographic platform, would aes-gcm be better maintained by tools because of the above?

@trmartin4 trmartin4 requested a review from a team December 31, 2025 03:02
@trmartin4
Copy link
Member Author

@trmartin4 / @Thomas-Avery aes-gcm is only used by the tools team for the chromium importer feature, in order to decrypt chromium imports. KM does not use it in any capacity or offer any functionality on top of it, despite it being a crypto dependency, and there is no good way for KM to maintain it / test functionality if updates come through. For instance, if the aes-gcm crate has a breaking change, would KM be expected to update tools code to update the dep?

While KM does build the cryptographic platform, would aes-gcm be better maintained by tools because of the above?

That makes sense. I've moved it and tagged Tools on the PR as well so they're aware.

@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.25%. Comparing base (11b5342) to head (de940e7).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #18153      +/-   ##
==========================================
- Coverage   42.26%   42.25%   -0.01%     
==========================================
  Files        3599     3599              
  Lines      104516   104516              
  Branches    15776    15776              
==========================================
- Hits        44171    44164       -7     
- Misses      58465    58472       +7     
  Partials     1880     1880              

☔ View full report in Codecov by Sentry.
📢 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants