Feat/passkey controller messenger migration#9548
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a96c601. Configure here.
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eade524275
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!password) { | ||
| throw new Error(PasskeyControllerErrorMessage.EnrollmentPasswordRequired); |
There was a problem hiding this comment.
Return a coded error for missing enrollment password
When onboarding is complete and protectVaultKeyWithPasskey is called without a password, this path throws a plain Error instead of a PasskeyControllerError with a stable code. Product callers are directed to key off PasskeyControllerError.code for expected operational failures, so they cannot reliably distinguish the normal “prompt for password” case from unexpected malformed input errors; add a coded controller error for EnrollmentPasswordRequired.
Useful? React with 👍 / 👎.
| - `exportAccountsWithPasskey` | ||
| - `PasskeyControllerOptions` with required `getIsOnboardingCompleted` constructor callback for enrollment step-up gating. ([#9548](https://github.com/MetaMask/core/pull/9548)) | ||
| - Added new error constants, `PasskeyControllerErrorCode.VaultKeyRenewalFailed`, `PasskeyControllerErrorMessage.VaultKeyRenewalFailed` and `PasskeyControllerErrorMessage.EnrollmentPasswordRequired`. ([#9548](https://github.com/MetaMask/core/pull/9548)) | ||
| - `@metamask/keyring-controller` dependency for KeyringController messenger action types used during orchestration |
There was a problem hiding this comment.
Add the missing PR link to this changelog entry
The AGENTS.md changelog rule says each changelog entry should be followed by links to the PR(s) that introduced the change, but this new dependency entry has no PR link while the neighboring entries cite #9548. Please add the PR link so the unreleased changelog remains traceable and validation/release review does not have to infer where this change came from.
Useful? React with 👍 / 👎.
|
|
||
| Manages passkey-based vault key protection using [WebAuthn](https://www.w3.org/TR/webauthn-3/). Orchestrates the full passkey lifecycle: generating WebAuthn ceremony options, verifying authenticator responses, and protecting/retrieving the vault encryption key via AES-256-GCM wrapping with HKDF-derived keys. | ||
|
|
||
| For the messenger migration design and extension wiring checklist, see [docs/messenger-migration.md](./docs/messenger-migration.md). |
There was a problem hiding this comment.
Restore the referenced migration document
This new README link points to ./docs/messenger-migration.md, but I searched the package (rg --files packages/passkey-controller | rg 'messenger-migration|docs/') and there is no docs directory or migration document. Anyone following the setup guidance lands on a broken link, so either add the document or update/remove the reference.
Useful? React with 👍 / 👎.
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |

Explanation
Motivation
Update
@metamask/passkey-controllerto messenger oriented controller. This is the first step of migration to use UI messengers.Current state
PasskeyControllertoday handles WebAuthn ceremonies and vault-key wrap/unwrap only. Product flows (unlock, password change, passkey removal, SRP/account export) are orchestrated in the extension'sMetaMaskController, which callsKeyringControllerdirectly alongside passkey helpers. That splits ownership across layers and makes messenger-based wiring harder.Solution
This PR makes
PasskeyControllerthe single owner of passkey product flows by orchestrating a fixed set ofKeyringControllermessenger actions inside the controller.Referenced document: https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/401773985849/PasskeyController+messenger+migration
New orchestrated methods (each exposed as a messenger action):
unlockWithPasskeysubmitEncryptionKeyprotectVaultKeyWithPasskeyverifyPassword(when onboarding complete),exportEncryptionKeychangePasswordWithPasskeyVerificationexportEncryptionKey→changePassword→exportEncryptionKey→ re-wrap passkey blobremovePasskeyWithPasskeyVerification/removePasskeyWithPasswordVerificationexportSeedPhraseWithPasskeyexportSeedPhrasewith{ encryptionKey }exportAccountsWithPasskeyexportAccountwith{ encryptionKey }Low-level methods (
retrieveVaultKeyWithPasskey,renewVaultKeyProtection) remain available for advanced composition.Breaking changes
PasskeyControllerconstructor now requiresgetIsOnboardingCompleted: () => boolean(onboarding is not read via messenger; core deliberately diverges from the extension doc'sOnboardingControllerdependency).protectVaultKeyWithPasskeyno longer acceptsvaultKey; it fetches the current encryption key viaKeyringController:exportEncryptionKey. When onboarding is complete,passwordis required for step-up (verifyPassword) before export.removePasskey/PasskeyController:removePasskeyremoved; useremovePasskeyWithPasskeyVerification,removePasskeyWithPasswordVerification, orclearState.PasskeyControllerMessengermust allow the KeyringController actions listed in the README.Non-obvious design choices
OnboardingControlleris extension-only; integrators pass() => onboardingController.state.completedOnboarding.changePassword, the passkey is removed andVaultKeyRenewalFailedis thrown (password change is not rolled back).async-mutex(runExclusive) to prevent concurrent vault/keyring races withinPasskeyController.exportEncryptionKey,changePassword,exportSeedPhrase,exportAccount) assert the keyring is unlocked.unlockWithPasskeycallssubmitEncryptionKey; other flows assume the integrator has already unlocked the keyring (typical for in-session settings/export flows). Post-onboarding enrollment usesverifyPasswordfor step-up but does not unlock — the vault must already be unlocked whenexportEncryptionKeyruns.Dependencies added
@metamask/keyring-controller— types for allowed messenger actions used during orchestration.async-mutex— serializes orchestrated passkey operations.Other changes
types.ts.tests/mocks/passkey-controller-messenger.ts.docs/messenger-migration.mdChecklist
Note
High Risk
Touches vault encryption, password change, unlock, and sensitive export paths with breaking API changes; failed passkey re-wrap after password change removes the passkey without rolling back the new password.
Overview
PasskeyController becomes the single layer for passkey product flows by calling KeyringController messenger actions (unlock, enroll, password change, removal, SRP/account export) instead of leaving that orchestration to the extension.
New orchestrated APIs and messenger actions include
unlockWithPasskey,changePasswordWithPasskeyVerification,removePasskeyWithPasskeyVerification/removePasskeyWithPasswordVerification,exportSeedPhraseWithPasskey, andexportAccountsWithPasskey. Orchestrated async work is serialized with an async-mutex to avoid vault/keyring races.Breaking: constructor now requires
getIsOnboardingCompleted;protectVaultKeyWithPasskeydropsvaultKeyand pulls the encryption key viaKeyringController:exportEncryptionKey(optionalpasswordstep-up when onboarding is complete); publicremovePasskeyis removed in favor of the step-up removal methods.PasskeyControllerMessengermust allow the documented KeyringController actions. Adds@metamask/keyring-controllerandasync-mutexdependencies.Reviewed by Cursor Bugbot for commit 1ed0878. Bugbot is set up for automated code reviews on this repo. Configure here.