PM-8353 MacOS passkey provider

[abergs]

🎟️ Tracking

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

📔 Objective

This PR serves as the long running feature branch for passkey provider support.
MacOS Passkey MVP Proposal
Explainer for other teams
Testing Setup
Passkey Workflow Videos

📸 Screenshots

⏰ 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

[codecov]

Codecov Report

❌ Patch coverage is 34.28571% with 299 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.27%. Comparing base (9e6d0cc) to head (a38bc28).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../src/autofill/services/desktop-autofill.service.ts	2.24%	87 Missing ⚠️
...l/services/desktop-fido2-user-interface.service.ts	0.00%	59 Missing ⚠️
...s/desktop/desktop_native/macos_provider/src/lib.rs	0.00%	37 Missing ⚠️
...src/platform/main/autofill/native-autofill.main.ts	0.00%	27 Missing ⚠️
apps/desktop/desktop_native/napi/src/lib.rs	0.00%	19 Missing ⚠️
...esktop/src/autofill/guards/reactive-vault-guard.ts	0.00%	19 Missing ⚠️
...tofill/modal/credentials/fido2-create.component.ts	77.64%	18 Missing and 1 partial ⚠️
...utofill/modal/credentials/fido2-vault.component.ts	86.66%	7 Missing and 1 partial ⚠️
apps/desktop/src/platform/popup-modal-styles.ts	0.00%	6 Missing ⚠️
apps/desktop/src/app/app-routing.module.ts	0.00%	4 Missing ⚠️
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13963      +/-   ##
==========================================
+ Coverage   41.23%   41.27%   +0.03%     
==========================================
  Files        3543     3546       +3     
  Lines      101963   102291     +328     
  Branches    15295    15338      +43     
==========================================
+ Hits        42048    42224     +176     
- Misses      58152    58300     +148     
- Partials     1763     1767       +4     

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

[github-actions]

Checkmarx One – Scan Summary & Details – 9a39ec3a-8799-4db9-8b97-cbda79a854b5

New Issues (7)

Checkmarx found the following issues in this Pull Request

Severity	Issue	Source File / Package	Checkmarx Insight
	CVE-2025-12727	Npm-electron-37.7.0	details Description: Inappropriate implementation in V8 in Google Chrome prior to 142.0.7444.137 allowed a remote attacker to potentially exploit heap corruption via a ... Attack Vector: NETWORK Attack Complexity: LOW ID: QuIjKQo3SsmQhRnzEmkohxQ9Mwk3bPmcDjDNUmk8EV0%3D Vulnerable Package
	CVE-2025-12907	Npm-electron-37.7.0	details Description: Insufficient validation of untrusted input in Devtools in Google Chrome prior to 140.0.7339.80 allowed a remote attacker to execute arbitrary code ... Attack Vector: NETWORK Attack Complexity: LOW ID: XFYFwxD6vqS8uMhYbKyUkRFARfKMmvRdCbzZoGeN%2B78%3D Vulnerable Package
	CVE-2025-11215	Npm-electron-37.7.0	details Description: Off-by-one Error in V8 in Google Chrome prior to 141.0.7390.54 allowed a remote attacker to perform an Out-of-bounds Memory Read via a crafted HTML... Attack Vector: NETWORK Attack Complexity: LOW ID: NVT%2F4Rn7JRRsnglE9fReLcdWh3gmCJWXjT%2FdsKSdgu4%3D Vulnerable Package
	CVE-2025-11216	Npm-electron-37.7.0	details Recommended version: 39.2.0 Description: Inappropriate implementation in Storage in Google Chrome on Mac prior to 141.0.7390.54 allowed a remote attacker to perform domain spoofing via a c... Attack Vector: NETWORK Attack Complexity: LOW ID: J%2B%2FQEE%2B3LWjYq5l%2BtEK9BslgLiBPFQxCVs1xOKHxT%2Fs%3D Vulnerable Package
	CVE-2025-12728	Npm-electron-37.7.0	details Description: Inappropriate implementation in Omnibox in Google Chrome on Android prior to 142.0.7444.137 allowed a remote attacker who convinced a user to engag... Attack Vector: NETWORK Attack Complexity: HIGH ID: p1lasSHcT6O44xENvA5OFRlx0awvOjqdzTBOoL3zFjE%3D Vulnerable Package
	CVE-2025-12729	Npm-electron-37.7.0	details Description: Inappropriate implementation in Omnibox in Google Chrome on Android prior to 142.0.7444.137 allowed a remote attacker who convinced a user to engag... Attack Vector: NETWORK Attack Complexity: HIGH ID: vUBiL%2Fz8YN0svAzm9%2B7Cli3mUY3HjG3L9EQm3U9anZY%3D Vulnerable Package
	CVE-2025-11219	Npm-electron-37.7.0	details Recommended version: 39.2.0 Description: Use After Free in V8 in Google Chrome prior to 141.0.7390.54 allowed a remote attacker to potentially perform Out-of-bounds Memory Access via a cra... Attack Vector: NETWORK Attack Complexity: HIGH ID: ajxzdEyNPqSogxVksc3%2B7Uic9oPEIhGj7bKYvCpewA8%3D Vulnerable Package

[claude]

❌ Angular Control Flow: This PR should use the modern @if / @else syntax instead of *ngIf with ng-template.

Suggested change

	<div *ngIf="(ciphers$ | async)?.length === 0; else hasCiphers">
	<div @if ((ciphers$ | async)?.length === 0) {

This applies to the entire template structure. The modern syntax improves readability and is the recommended pattern for new code. Since this is a new component being added, it should follow current best practices from the start.

[claude]

❌ Angular Control Flow: Use @for instead of *ngFor for iteration.

Suggested change

	<bit-item *ngFor="let c of ciphers$ | async" class="">
	<bit-item @for (c of ciphers$ | async; track c.id) {

The @for syntax is the modern Angular pattern and should be used in new components. Note the required track expression for performance optimization.

[claude]

⚠️ Acknowledged tech debt: The FIXME comment correctly identifies that unwraps should be removed. These can cause panics and crash the extension.

The comment acknowledges this is a known issue. Consider creating a follow-up ticket to address this before the feature exits beta/MVP status, as production crashes from panics would significantly impact user experience.

[claude]

⚠️ Force unwrap on optional: newClient! will crash if the connection loop completes without establishing a connection.

guard let client = newClient else {
    logger.error("[autofill-extension] Failed to create client after retries")
    throw NSError(domain: "com.bitwarden.desktop", code: 1, userInfo: nil)
}
self.client = client
return client

This prevents crashes and provides better error messaging to users.

[claude]

⚠️ Inconsistent optional typing: session is typed as DesktopFido2UserInterfaceSession | undefined but initialized to null.

Suggested change

	session?: DesktopFido2UserInterfaceSession = null;
	session?: DesktopFido2UserInterfaceSession;

Use either undefined (via ?) or null, but not both. The ? syntax is preferred and matches how it's checked elsewhere in the code.

[claude]

⚠️ Same inconsistent typing: Use ? for optional instead of = null.

Suggested change

	session?: DesktopFido2UserInterfaceSession = null;
	session?: DesktopFido2UserInterfaceSession;

[claude]

💭 Documentation needed: These custom utility classes lack documentation about their purpose and when to use them.

Consider adding a comment block:

/**
 * macOS window dragging utilities
 * .tw-app-region-drag - Makes the element draggable for window movement (macOS titlebar)
 * .tw-app-region-no-drag - Prevents dragging on interactive elements within draggable areas
 */

This helps future developers understand the purpose of these non-standard Tailwind classes.

[claude]

💭 Localization process: Since this adds new user-facing strings, confirm these have been or will be submitted for translation via Crowdin.

Per the earlier comment thread, design approval was obtained. Just verifying the i18n process is also complete or planned.

[claude]

⚠️ Typo in variable name: xPostionOffset should be xPositionOffset

[claude]

💭 TypeScript inconsistency: The type annotation DesktopFido2UserInterfaceSession | undefined suggests it could be undefined, but it's initialized to null. Consider using null in the type annotation instead for consistency: DesktopFido2UserInterfaceSession | null

[claude]

💭 Same as in fido2-create.component.ts: initialized to null but typed as undefined. Consider using consistent null/undefined semantics.

[claude]

⚠️ As noted in the FIXME comment, unwraps should be removed. While this specific panic location may be acceptable during initialization, consider using .expect() with descriptive messages for better debugging. The current comment acknowledges this but the issue persists.

[claude]

❌ Observable anti-pattern: Using map with an async function doesn't wait for the promise to complete. The observable will emit the unresolved Promise, not the result.

Fix: Change map to switchMap:

async updateCredential(cipher: CipherView): Promise<void> {
  this.logService.info("updateCredential");
  await firstValueFrom(
    this.accountService.activeAccount$.pipe(
      switchMap(async (a) => {
        if (a) {
          const encCipher = await this.cipherService.encrypt(cipher, a.id);
          await this.cipherService.updateWithServer(encCipher);
        }
      }),
    ),
  );
}

This ensures the async operations complete before the observable completes.

[claude]

⚠️ Potential crash: Force unwrapping newClient! could crash if all connection attempts fail. Consider handling this case gracefully:

guard let client = newClient else {
    logger.error("[autofill-extension] Failed to establish connection after all retries")
    // Handle failure appropriately - perhaps throw an error or return a default/stub client
    fatalError("Unable to connect to Bitwarden Desktop")
}
self.client = client
return client

Or make the return type optional and handle nil at call sites.

[claude]

⚠️ Potential error: Consider validating lastRegistrationRequest.userHandle before conversion to prevent potential crashes:

const userHandle = lastRegistrationRequest.userHandle 
  ? Fido2Utils.bufferToString(new Uint8Array(lastRegistrationRequest.userHandle))
  : null;

if (!userHandle) {
  this.logService.error("Invalid userHandle in registration request");
  return;
}

[claude]

🎨 Minor improvement: Typo in variable name and could be extracted to class-level constant:

// At class level:
private readonly POSITION_X_OFFSET = 100;

// In function:
return {
  x: Math.round(position.x + this.POSITION_X_OFFSET),
  y: Math.round(position.y),
};

Note: "xPostionOffset" → "POSITION_X_OFFSET"

[claude]

❌ Security concern: This debug log outputs the full converted registration request which may contain sensitive user data (userHandle, userName, etc.).

While this doesn't log decrypted vault data directly, it does log user identification information that should not appear in production logs.

Recommendation: Either:

1. Remove the second debug log entirely (the first on line 212 already logs the basic info)
2. Gate this behind a development-only check
3. Create a sanitized version that logs only non-sensitive fields (rpId, algorithms count, etc.)

// Option 1: Remove entirely
this.logService.debug("listenPasskeyRegistration", clientId, sequenceNumber, request);

// Option 2: Development only
if (isDevelopment()) {
  this.logService.debug("listenPasskeyRegistration2", this.convertRegistrationRequest(request));
}

// Option 3: Sanitized logging
this.logService.debug("listenPasskeyRegistration2", {
  rpId: request.rpId,
  algorithmCount: request.supportedAlgorithms.length,
  hasExcludedCredentials: request.excludedCredentials.length > 0
});

[claude]

❌ Similar concern: This logs the full assertion request which includes credential identifiers and user context.

Recommendation: Consider reducing to essential info only:

this.logService.debug("listenPasskeyAssertion", {
  clientId,
  sequenceNumber,
  rpId: request.rpId,
  credentialCount: request.allowedCredentials?.length ?? 0
});

[claude]

⚠️ Force unwrap risk: While the retry logic makes this unlikely to fail, there's still a path where all 20 connection attempts fail and newClient could be nil, causing a crash here.

Recommendation: Use safer optional handling:

guard let client = newClient else {
    logger.error("[autofill-extension] Failed to establish client after \(maxRetries) attempts")
    // Could throw an error or return a failed client state
    fatalError("Unable to establish IPC connection to Bitwarden Desktop")
}
let connectionStatus = client.getConnectionStatus()

This makes the failure mode explicit rather than risking an unexpected crash.

[claude]

⚠️ Same force unwrap concern: This should use guard or optional chaining to handle the nil case gracefully.

guard let client = self.client, let newClient = newClient else {
    logger.error("[autofill-extension] Client initialization failed")
    fatalError("Unable to establish IPC connection to Bitwarden Desktop")
}
return client ?? newClient

[claude]

💭 Question about subscription pattern: This implementation uses explicit subscribe with takeUntil(this.destroy$) which is valid, but I'm curious about the pattern choice.

Given ADR-0003's preference for the async pipe pattern, was there a specific reason not to use:

// In template:
<div *ngFor="let cipher of ciphers$ | async">

This would:

1. Handle subscription cleanup automatically
2. Be more consistent with the Observable Data Services pattern
3. Eliminate the need for manual BehaviorSubject management

Is the manual subscription needed for error handling or some other requirement I'm missing? Just want to understand the design decision.

[bw-ghapp]

⚠️ Changes in this PR impact the Autofill experience of the browser client ⚠️

BIT has tested the core experience with these changes and the feature flag configuration used by vault.bitwarden.com

Caution

Unfortunately, one or more of these tests failed. 😞

Please resolve the failure before merging; reach out to @bitwarden/team-autofill-dev if you'd like help.

You can view the detailed results of the tests here.

[bw-ghapp]

⚠️ Changes in this PR impact the Autofill experience of the browser client ⚠️

BIT has tested the core experience with these changes and all feature flags disabled.

Caution

Unfortunately, one or more of these tests failed. 😞

Please resolve the failure before merging; reach out to @bitwarden/team-autofill-dev if you'd like help.

You can view the detailed results of the tests here.