feat: detect and repair lagging recovery escrows

packages/mfa-recovery-controller/ARCHITECTURE.md

@@ -74,8 +74,11 @@ Public surface (all exposed as messenger actions): `register`,
 [Recovery Lock](README.md#recovery-lock)).
 
 `getIdentifiers` takes an AAL1 approval and returns the enrolled identifiers
-at the highest consistent escrow version as `epoch`, plus Auth's identifiers.
-A new device builds the `updateIdentifiers` set and `expectedEpoch` from it.
+and their escrow version as `epoch`, plus Auth's identifiers. A new device
+builds the `updateIdentifiers` set and `expectedEpoch` from it. Like a
+mutation, it requires every escrow and the same version on each
+(`replica_lagging`), so lagging escrows are repaired before Auth changes:
+repair copies identifiers that Auth must still approve.
 
 ## State machine
 
@@ -88,8 +91,8 @@ stateDiagram-v2
   authorizing --> idle: abort()
   authorizing --> writing: identifier auth succeeds, before first apply
   writing --> writing: more receipts, still missing replicas
-  writing --> idle: every configured escrow has a valid receipt
-  writing --> idle: every escrow rejects and none has a receipt
+  writing --> idle: every target escrow has a valid receipt
+  writing --> idle: every remaining escrow rejects
   writing --> writing: resume() retries only missing receipts
   authorizing --> writing: resume() authorizes then writes
 ```
@@ -104,7 +107,8 @@ Rules:
   failure remains resumable, while pre-write failures stay abortable.
 - `register` / `updateRecoverySecret` / `updateIdentifiers` require idle
   pending state (`pending_mutation`). `resume()` finishes a persisted
-  mutation and is a no-op when idle.
+  mutation. When idle it is a no-op, or repairs lagging replicas if given an
+  identifier.
 
 ### Pending state validation
 
@@ -114,7 +118,7 @@ Anything that fails is rejected rather than acted on:
 - Shape: phase, mutation fields, `newVersion = expectedVersion + 1`, payload
   keys for the operation, hex secret, at least two distinct identifiers, known
   identifier types.
-- `audiences` must equal the configured escrow ids, in order
+- `audiences` must be configured escrow ids, in configured order
   (`audience_mismatch`).
 - `payloadHash` must match the payload (`payload_mismatch`) and `requestHash`
   must match the mutation fields.
@@ -143,6 +147,8 @@ sequenceDiagram
   C->>M: register / updateRecoverySecret / updateIdentifiers
   M->>M: lock, reject if a mutation is already pending
   M->>E: isAvailable (every escrow required)
+  M->>E: getIdentifiers (AAL1); differing versions → replica_lagging
+  M->>M: update identifier not enrolled everywhere → identifier_not_enrolled
   M->>A: getAuthenticatedProfileId
   M->>M: expectedVersion from caller epoch (register uses 0)
   M->>M: build mutation, requestHash = hash(mutation fields)
@@ -151,9 +157,10 @@ sequenceDiagram
   M->>M: skip escrows with a stored receipt, re-verify stored receipts
   M->>E: isAvailable (remaining escrows)
   M->>M: reuse token, or re-issue it if exp <= now()
-  opt updateIdentifiers with SIWE identifiers
-    M->>E: getIdentifiers (enrolled set at expectedVersion)
-  end
+  M->>E: getIdentifiers (remaining escrows)
+  M->>M: some escrow off expectedVersion → probe only those, no proofs
+  M->>M: else update identifier missing at expectedVersion → identifier_not_enrolled
+  M->>M: re-issued approval misses a payload identifier and every escrow moved → clear, mutation_superseded
   opt update identifier, or SIWE identifiers not yet enrolled
     M->>I: getKeyBoundIdentifierToken per identifier
     M->>E: generateChallenge + PoPs per remaining escrow (all required)
@@ -162,9 +169,12 @@ sequenceDiagram
   par write every remaining replica
     M->>E: applyMutation
   end
-  alt every escrow rejects and none has a receipt
+  alt every remaining escrow rejects, none has a receipt
     M->>M: clear pending → idle
     M-->>C: MutationRejectedError (start a new mutation)
+  else every remaining escrow rejects, others have receipts
+    M->>M: clear pending → idle
+    M-->>C: mutation_superseded (reread state)
   else
     M->>M: persist valid receipts
     alt a receipt failed verification
@@ -202,9 +212,12 @@ Receipts:
   target, and `escrow.verifyReceipt` accepts it (pinned receipt key).
 - Stored receipts are re-verified before every retry. A receipt for an
   escrow that is no longer configured fails with `unknown_receipt_escrow`.
-- `MutationRejectedError` is only surfaced when **every** remaining escrow
-  threw `MutationRejectedError` and no receipt exists yet; anything else may
-  have been applied and stays `writing`.
+- When **every** remaining escrow throws `MutationRejectedError`, the pending
+  mutation is cleared. With no receipts nothing was applied, and that error is
+  surfaced. With receipts the mutation was applied on some escrows and is
+  obsolete or already repaired elsewhere, so `mutation_superseded` is thrown
+  instead; reread state. Anything else may have been applied and stays
+  `writing`.
 
 Versioning:
 
@@ -215,10 +228,32 @@ Versioning:
 - updates take the caller-supplied `epoch` (a non-negative integer, else
   `invalid_epoch`) as `expectedVersion`, then `n → n+1`. Escrows reject a
   mismatch.
-- Mutation `audiences` is the full configured replica-id list, in that order.
-  Each replica should require an exact match to its configured set (a
-  cubist-only deploy is `["cubist"]`; adding another escrow means every replica
-  is rebuilt with the expanded list).
+- Mutation `audiences` is the configured replica-id list, in that order, or
+  the lagging subset for a repair. Each escrow requires its own id exactly
+  once (a cubist-only deploy is `["cubist"]`).
+- An escrow with no record reports version `0`. New mutations require every
+  escrow at the same version (`replica_lagging`).
+- That check runs before the lease, so another device may write first. Each
+  write rereads versions under the lease. Escrows off `expectedVersion` are
+  probed alone with no identifier proofs: they replay
+  this mutation's receipt or reject it. So it never forks the rest, and an
+  obsolete mutation clears even if its identifier or SIWE wallet is gone.
+- A re-issued approval drops identifiers removed from Auth, and escrows fail a
+  payload it does not cover. If every remaining escrow moved, the mutation can
+  no longer land, so it is cleared with `mutation_superseded` unsent.
+- An update's `identifier` must be enrolled on every target escrow
+  (`identifier_not_enrolled`), checked before approval and before each write:
+  escrows fail, rather than reject, an unknown one.
+- `resume(identifier)` with a pending update authorizes it with `identifier`
+  instead. Escrows accept any enrolled identifier, and `requestHash` does not
+  cover it.
+
+`resume(identifier)` with no pending mutation reads every escrow's version and
+identifiers. Escrows one version behind the highest get one mutation
+addressed only to them: `register` if they have no record,
+`updateIdentifiers` if their identifiers differ, otherwise
+`updateRecoverySecret` with the secret read from the highest escrow. Escrows
+further behind fail with `replica_corruption`.
 
 `resume()` is the same write path without creating a new mutation:
 authorizing pending gets a fresh token then replicates; writing pending
@@ -242,7 +277,9 @@ and `applyMutation`. `generateChallenge` and `getIdentifiers` take the
 operation's `authControllerToken`; the escrow keys the challenge to its
 profile. Escrow JSON is parsed from C2F stdout. A policy `Deny` on
 `applyMutation` becomes `MutationRejectedError`; any other failure is
-`escrow_request_failed` / `escrow_response_invalid`. `verifyReceipt` checks
+`escrow_request_failed` / `escrow_response_invalid`. Probes send no identifier
+authorizations, so `store::apply` must replay a receipt or deny a version
+mismatch before verifying them. `verifyReceipt` checks
 the mutation id, request hash, escrow id, `version === newVersion`,
 `receiptKeyId` (SHA-256 of the pinned receipt key), and the Ed25519 signature
 locally.
@@ -300,7 +337,8 @@ memory and must not log it.
 A replica whose `wrapKeyId` does not match its configured `wrapPublicKey`, or
 whose ciphertext fails to unwrap, counts as a failed read. Among successful
 reads, the highest `version` wins; every replica at that version must agree
-on `lastMutationId` and the secret bytes.
+on the secret bytes. `lastMutationId` is not compared, since a repaired
+replica holds the same secret under the repair's mutation.
 
 `getRecoverySecret` runs under the controller lock but does not check pending
 state. During `writing`, replicas may hold different versions; the read takes

packages/mfa-recovery-controller/CHANGELOG.md

@@ -12,12 +12,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Initial release of `MfaRecoveryController`, which manages MFA recovery flows and persists in-flight operations in its state ([#10022](https://github.com/MetaMask/core/pull/10022))
   - Requires recent, request-bound AAL2 approvals for writes and reads.
   - Sends the approval's identifiers to escrows as `approvedIdentifiers` with `register` and `updateIdentifiers` mutations, and rejects payload identifiers that are not exactly among them before writing ([#10883](https://github.com/MetaMask/core/pull/10883))
-  - Adds `getIdentifiers`, which returns the enrolled identifiers at the highest escrow version as `epoch` along with Auth's identifiers, for building `updateIdentifiers` calls ([#11149](https://github.com/MetaMask/core/pull/11149))
+  - Adds `getIdentifiers`, which returns the enrolled identifiers and their escrow version as `epoch` along with Auth's identifiers, for building `updateIdentifiers` calls ([#11149](https://github.com/MetaMask/core/pull/11149))
   - Passes the operation's AuthController token to `RecoveryEscrowProvider.generateChallenge`, and requires `RecoveryEscrowProvider.getIdentifiers` ([#11149](https://github.com/MetaMask/core/pull/11149))
   - Proves each SIWE identifier an escrow has not enrolled through `identifierAuthProvider`, including at `register`, instead of requiring it in `approvedIdentifiers`, and passes `RecoveryEscrowProvider.applyMutation` a list of identifier authorizations sharing one challenge ([#11151](https://github.com/MetaMask/core/pull/11151))
   - Rejects SIWE identifiers whose `namespace` is not their verifier's `domain:chainId` with `invalid_identifier` ([#11151](https://github.com/MetaMask/core/pull/11151))
   - Requires a `lockProvider` and adds `acquireLock`, `renewLock` and `releaseLock`; the lease is acquired or renewed before every approval, and its ID is persisted in `lockId` ([#10910](https://github.com/MetaMask/core/pull/10910))
   - Exports `RecoveryRegistrationLockProvider`, `RecoveryRegistrationLockProviderOptions`, `RecoveryRegistrationLockProviderMessenger` and `RecoveryLockProvider` for holding the lease in the recovery registration service ([#10910](https://github.com/MetaMask/core/pull/10910))
+  - Requires every escrow for `getIdentifiers`, as for mutations ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Rejects new mutations and `getIdentifiers` with `replica_lagging` when escrows are at different versions, and adds an optional `identifier` to `resume`; with no pending mutation, `resume(identifier)` copies the highest escrow record to escrows one version behind it ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Sends a resumed mutation only to escrows another write moved off its `expectedVersion`, if any, with no identifier proofs, so it cannot fork escrows behind it and clears even if its identifier or SIWE wallet is unavailable; `RecoveryEscrowProvider.applyMutation` must replay a stored receipt or reject a version mismatch before verifying `identifierAuthorizations` ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Rejects updates whose `identifier` is not enrolled on every target escrow with `identifier_not_enrolled`, before requesting approval and again before each write ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Authorizes a pending update with the `identifier` passed to `resume`, instead of the saved one, and saves it for later resumes ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Clears a superseded pending mutation with `mutation_superseded`: when every remaining escrow rejects it but others have receipts (instead of `MutationRejectedError`), or, without sending it, when its re-issued approval no longer covers the payload identifiers and every remaining escrow moved off `expectedVersion` ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Compares only the secret, not `lastMutationId`, when checking replicas at the highest version on reads ([#11200](https://github.com/MetaMask/core/pull/11200))
+  - Requires `RecoveryEscrowProvider.getIdentifiers` to report a missing record as `{ identifiers: [], version: 0 }`, so `getIdentifiers` returns `epoch: 0` before registration ([#11200](https://github.com/MetaMask/core/pull/11200))
   - Exports `MfaRecoveryController`, `getDefaultMfaRecoveryControllerState`, and the controller's action, event, messenger, state, and options types.
   - Exports `AuthApiRecoveryAuthProvider`, `AuthApiRecoveryAuthProviderOptions` and `AuthApiRecoveryAuthProviderMessenger` for requesting approvals through `AuthenticationController:requestMfaApproval`, using the profile id from `AuthenticationController:refreshCanonicalProfileId` ([#10883](https://github.com/MetaMask/core/pull/10883))
   - Exports `CubistEscrowProvider` and `CubistEscrowProviderOptions` for storing recovery secrets with CubeSigner.

packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md

@@ -71,6 +71,8 @@ sequenceDiagram
   Auth-->>C: JWT bound to requestHash
   C->>Cubist: isAvailable
 
+  C->>Cubist: getIdentifiers(JWT)
+  Cubist-->>C: version and enrolled identifiers
   opt updateIdentifiers adding SIWE identifiers
     C->>Cubist: getIdentifiers(JWT)
     Cubist-->>C: enrolled identifiers, version
@@ -96,10 +98,10 @@ Auth does not vouch for SIWE wallets, so `approvedIdentifiers` need not
 include them; each new wallet proves control instead. In step 1 the primary
 account signs a SIWE proof for `register`.
 
-If an escrow refuses the mutation (`MutationRejectedError`) and every replica
-does the same, the controller clears the pending mutation. A transport failure
-or a partial receipt keeps it and throws `incomplete_mutation`; call `resume()`
-to finish that write.
+If every remaining replica refuses the mutation (`MutationRejectedError`), the
+controller clears the pending mutation. It throws `mutation_superseded` instead
+if another replica already has a receipt; reread state. Any other failure keeps
+it and throws `incomplete_mutation`; call `resume()` to finish that write.
 
 ## Proof of possession
 

packages/mfa-recovery-controller/README.md

@@ -3,7 +3,17 @@
 Coordinates MFA recovery through injected auth and escrow providers.
 
 Pending mutations are encrypted in controller state. `resume()` completes them;
-`abort()` is allowed only before writing. See [Architecture](ARCHITECTURE.md).
+`abort()` is allowed only before writing. If the identifier an update started
+with can no longer authenticate, pass another enrolled one to
+`resume(identifier)`. See [Architecture](ARCHITECTURE.md).
+
+## Replica Repair
+
+Mutations and `getIdentifiers()` fail with `replica_lagging` when escrows are
+at different versions, for example after an interrupted write whose pending
+state was lost. Call `resume(identifier)` under the recovery lock: with no
+pending mutation, it copies the highest record to escrows one version behind
+it. Then retry the mutation.
 
 ## Recovery Lock
 

packages/mfa-recovery-controller/src/MfaRecoveryController-method-action-types.ts

@@ -94,9 +94,10 @@ export type MfaRecoveryControllerGetRecoverySecretAction = {
 };
 
 /**
- * Reads the enrolled identifiers from available escrows at the highest
- * consistent version, with Auth's current identifiers. Needs no recent MFA,
- * so a new device can build an {@link updateIdentifiers} call from it.
+ * Reads the enrolled identifiers from every escrow, with Auth's current
+ * identifiers. Needs no recent MFA, so a new device can build an
+ * {@link updateIdentifiers} call from it. Throws `replica_lagging` when
+ * escrows are at different versions.
  *
  * @returns Enrolled identifiers, `epoch`, and Auth's identifiers.
  */
@@ -106,7 +107,13 @@ export type MfaRecoveryControllerGetIdentifiersAction = {
 };
 
 /**
- * Completes a persisted pending mutation, if any.
+ * Completes a persisted pending mutation. With none pending and an
+ * `identifier`, copies the highest escrow record to escrows one version
+ * behind it; pass an identifier when a mutation fails with
+ * `replica_lagging`.
+ *
+ * @param identifier - Enrolled identifier that authorizes the repair, or
+ * replaces the one authorizing a pending update (e.g. if it was lost).
  */
 export type MfaRecoveryControllerResumeAction = {
   type: `MfaRecoveryController:resume`;