feat(settings): add passkey wrap creation by vpomerleau · Pull Request #21192 · mozilla/fxa

vpomerleau · 2026-09-11T18:29:18Z

Because

Passwordless Sync needs kB sealed to a passkey's PRF output before a wrap can be stored.
Callers need outcomes by errno: the wrap endpoints answer 401 and 404 for more than one condition each.

This pull request

Adds lib/passkeys/wrap/: createPasskeyWrap seals kB to the PRF output and stores the
envelope under an mfa:passkey proof; usePasskeyWrapCreation adds a loading flag.
Reads the account id from the proof's sub.
Reopens each envelope before storing it.
Maps server errnos to a closed failure union, passing errno/code/retryAfter through.
Zeroes kB and the PRF output after every seal attempt.

Issue that this pull request solves

Closes: FXA-14425

Checklist

Put an x in the boxes that apply

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).
I have manually reviewed all AI generated code.

How to review (Optional)

Key files/areas to focus on: wrap/creation.ts; the hook is 35 lines.
Suggested review order: creation.ts → use-passkey-wrap-creation.ts → creation.test.ts.
Risky or complex parts: the seal → reopen → zero ordering, and wrap_conflict (errno 235),
which the client cannot tell apart from a stale wrap.

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Supersedes #21187. That branch held sealed envelopes client-side to survive a lost response;
a re-seal gets errno 235 either way and the server already flags stale wraps, so the retry
state is gone.

Two deviations from the ticket: uid comes from the proof's sub and no session token is
needed; kB is left intact on the two pre-flight rejections (prf_unsupported,
proof_invalid) so sign-in can continue.

Deep-import lib/passkeys/wrap; it is not in lib/passkeys/index.ts because creation.ts
pulls in the HPKE suite, which builds a CipherSuite at module scope.

Because: - Passwordless Sync needs kB sealed to a passkey's PRF output before a wrap can be stored. This commit: - Adds createPasskeyWrap, sealing kB into a wrap envelope bound to the account the mfa:passkey proof names and storing it under that proof. - Reopens the envelope before storing it, catching platform crypto that seals what it cannot unseal. - Maps server errnos to distinct failure reasons; wrap_conflict means a wrap already exists, from a lost response or from before a key rotation. - Zeroes kB and the PRF output once sealing has been tried. - Adds usePasskeyWrapCreation, a loading flag around the call. Closes #FXA-14425

Copilot

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

packages/fxa-settings/src/lib/passkeys/wrap/creation.test.ts:250

The pre-flight contract says both kB and prfOut remain untouched for prf_unsupported and proof_invalid, but the input tests only assert that kB is preserved (the parametrized rejection test does not inspect either buffer, and the proof-invalid test checks only kB). Add exact prfOut preservation assertions for both early-return paths so the secret-handling contract is covered.

    it('leaves kB intact when the passkey cannot hold a wrap', async () => {
      const input = { ...args(), prfOut: undefined };

      await createPasskeyWrap(authClient(), input);

      expect(input.kB).toEqual(MOCK_KB);

packages/fxa-settings/src/lib/passkeys/wrap/creation.ts:19

This public type comment lists only 401/403/404, but wrap creation also returns distinct 409/429 (and WAF 406) outcomes. That status list is misleading for callers; describe the general status reuse instead of enumerating an incomplete set.

 * Why a wrap could not be stored. The server answers 401, 403 and 404 for more
 * than one condition each, so callers branch on these rather than on status.

packages/fxa-settings/src/lib/passkeys/wrap/use-passkey-wrap-creation.ts:29

If createWrap is invoked concurrently, the first request's finally sets this shared flag to false while the second request is still pending. A caller can therefore re-enable its submit control and start another wrap before all requests finish; track the number of in-flight calls (or enforce single-flight) and derive isLoading from that state.

      } finally {
        setIsLoading(false);
      }

packages/fxa-settings/src/lib/passkeys/wrap/use-passkey-wrap-creation.ts:28

This callback can be entered more than once while the first request is pending. Each invocation generates a different envelope for the create-only endpoint, so one call stores a wrap and the other returns wrap_conflict; the first finally also sets isLoading to false while the other call is still running. Guard or coalesce in-flight calls (as usePasskeySignIn does with inFlight at lib/passkeys/signin-flow.ts:344-347) so a double invocation cannot turn a successful creation into a spurious conflict.

      setIsLoading(true);
      try {
        return await createPasskeyWrap(authClient, args);
      } finally {
        setIsLoading(false);

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

+    refuse(
+      Object.assign(new Error('nope'), { errno: ERRNO.PASSKEY_NOT_FOUND })
+    );
+    await act(() => pending);


nshirley · 2026-09-11T20:38:10Z

+    ['a newly stored wrap', true],
+    ['an identical wrap already stored', false],
+  ])('reports %s as created=%s', async (_label, created) => {
+    createPasskeyWrapMock.mockResolvedValue({ created });


I was confused why you'd be mocking the function you're testing, but I see this is the auth-client call inside the function under test. Maybe something like createPasskeyWrapApiMock helps to differentiate the two at a glance?

nshirley · 2026-09-11T20:46:42Z

+    envelope = await createWrapEnvelope({ kB, prfOut, uid, credentialId });
+    // Sealing never runs the open half. A platform whose EC export diverges
+    // (see `key-wrap.ts`) seals well-formed envelopes that never open.
+    const recovered = await openWrapEnvelope({


Smart, running an open to make sure we can open the thing that was just sealed!

nshirley · 2026-09-11T21:58:16Z

+ * @throws on a wrong-width `kB` — a caller bug, not an outcome.
+ */
+export async function createPasskeyWrap(
+  authClient: PasskeyWrapAuthClient,


maybe a naive question with how settings is setup, but any reason to not just make this AuthClient class type?

oh, I see, looks like it's for helping out tests

nshirley · 2026-09-11T22:12:02Z

+      credentialId,
+    });
+    const matches = bytesEqual(recovered, kB);
+    recovered.fill(0);


I probably missed it, but we should have a test to make sure this also gets zeroed on success and failure. I'm wondering too if it might make sense to move the fill into the finally. That way if anything gets added between the creation of the recovered const and it being filled and that thing can throw (some new function etc), then recovered could leak

nshirley · 2026-09-11T22:21:04Z

+    const cause = causeOf(err);
+    if (failure === 'unexpected') {
+      Sentry.captureException(new Error('passkey-wrap-store error'), {
+        tags: { errno: String(cause?.errno ?? 'none') },


nshirley · 2026-09-11T22:29:23Z

+  }
+}
+
+function toFailure(err: unknown): PasskeyWrapFailure {


Is this a common thing to have to do in settings, essentially creating a mapping of errno to string values that can be handed back to I'm assuming a component? It feels like, if it's needed by settings, then the API should be returning it so you don't have to do this mapping

nshirley · 2026-09-11T22:40:02Z

+  'errno' | 'code' | 'retryAfter'
+>;
+
+export type CreatePasskeyWrapResult =


Really like the use of discriminated unions!

nshirley · 2026-09-11T23:03:33Z

+  }
+}
+
+function causeOf(err: unknown): PasskeyWrapCause | undefined {


This and the toFailure might make sense to export (or move to another module) just so they can be tested in isolation. They require a mock client to test by reaching through the createPasskeyWrap function to get to this point.

Then, just a single wiring test could remain for createPasskeyWrap to ensure the happy path and that it's correctly calling the functions, but you don't have to assert all the logic for them here.

However, to be fair, the current tests work and are passing! So, dealers choice 🙂

vpomerleau requested a balanced review from Copilot September 11, 2026 18:30

Copilot started reviewing on behalf of vpomerleau September 11, 2026 18:30 View session

This comment was marked as outdated.

Sign in to view

vpomerleau force-pushed the FXA-14425-wrap-simple branch from ae36b6b to 493c9eb Compare September 11, 2026 19:41

vpomerleau requested a lite review from Copilot September 11, 2026 19:42

Copilot started reviewing on behalf of vpomerleau September 11, 2026 19:43 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

Comment thread packages/fxa-settings/src/lib/passkeys/wrap/use-passkey-wrap-creation.test.tsx

refuse(

Object.assign(new Error('nope'), { errno: ERRNO.PASSKEY_NOT_FOUND })

);

await act(() => pending);

nshirley reviewed Sep 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(settings): add passkey wrap creation - #21192

feat(settings): add passkey wrap creation#21192
vpomerleau wants to merge 1 commit into
mainfrom
FXA-14425-wrap-simple

vpomerleau commented Sep 11, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

nshirley Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vpomerleau commented Sep 11, 2026

Because

This pull request

Issue that this pull request solves

Checklist

How to review (Optional)

Screenshots (Optional)

Other information (Optional)

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants