Skip to content

Small fix for ssh namespace mismatch#20502

Open
coltonhurst wants to merge 2 commits intomainfrom
dn/ssh-v2-namespace-mismatch
Open

Small fix for ssh namespace mismatch#20502
coltonhurst wants to merge 2 commits intomainfrom
dn/ssh-v2-namespace-mismatch

Conversation

@coltonhurst
Copy link
Copy Markdown
Member

@coltonhurst coltonhurst commented May 4, 2026

🎟️ Tracking

None

📔 Objective

This PR aims to fix a potential case mismatch in the ssh agent v1 and v2 "namespace" implementations. It may not be accurate, but here is the concept:

Line 59 of clients/apps/desktop/src/autofill/components/approve-ssh-request.ts checks the namespace string, currently matching on "git" for v1. However, v2's SIGNamespace.Git enum variant from clients/apps/desktop/desktop_native/napi/src/sshagent_v2.rs ends up being "Git", according to line 308 in clients/apps/desktop/desktop_native/napi/index.d.ts.

"Git" won't match here, so actioni18nKey will end up being "sshActionSign", rather than "sshActionGitSign".

There may be a better way to fix this, or this may be the intended behavior, and this PR is not needed. Just throwing this PR up as a starting point for conversation, and as a potential fix.

If the bug identified is accurate, then we'll want to verify this doesn't break anything on the ssh v1 side.

@coltonhurst coltonhurst self-assigned this May 4, 2026
@coltonhurst coltonhurst requested a review from a team as a code owner May 4, 2026 19:08
@coltonhurst coltonhurst changed the title Small fix for sshv2 namespace mismatch Small fix for ssh namespace mismatch May 4, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.11%. Comparing base (1c078bc) to head (96d0b7b).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #20502   +/-   ##
=======================================
  Coverage   47.11%   47.11%           
=======================================
  Files        3951     3951           
  Lines      119732   119732           
  Branches    18349    18349           
=======================================
  Hits        56410    56410           
  Misses      59086    59086           
  Partials     4236     4236           

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

Comment thread apps/desktop/src/autofill/components/approve-ssh-request.ts Outdated
@coltonhurst coltonhurst added the ai-review Request a Claude code review label May 4, 2026
@coltonhurst coltonhurst requested a review from a team as a code owner May 4, 2026 21:00
@coltonhurst coltonhurst requested a review from dani-garcia May 4, 2026 21:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR fixes the v1/v2 SSH agent namespace casing mismatch by switching the napi-rs string enum to camelCase serialization (#[napi(string_enum = "camelCase")]). The v2 SIGNamespace::Git enum now serializes as "git" (matching v1's raw protocol bytes per PROTOCOL.sshsig), so the existing namespace === "git" check in approve-ssh-request.ts works correctly for both code paths. The auto-generated index.d.ts was updated in lockstep. I verified there are no other TypeScript consumers comparing against the previous PascalCase values, so the change is well-scoped with no breaking impact.

Code Review Details

No findings.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 4, 2026

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.

3 participants