From a94659c748be7b18f8beeee7fcae1cee1fca888b Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 17:30:17 +0800 Subject: [PATCH 1/7] hot-fix: fixed skipping metadata write on toprf creation retry --- .../src/SeedlessOnboardingController.test.ts | 14 ++++++- .../src/SeedlessOnboardingController.ts | 39 +++++++++++++++---- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 3ff1b8651a8..20f5f936b49 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1470,6 +1470,7 @@ describe('SeedlessOnboardingController', () => { .mockResolvedValueOnce(); // encrypt and store the secret data const mockSecretDataAdd = handleMockSecretDataAdd(); + const retryMockSecretDataAdd = handleMockSecretDataAdd(); const authenticateSpy = jest .spyOn(toprfClient, 'authenticate') @@ -1486,6 +1487,7 @@ describe('SeedlessOnboardingController', () => { ); expect(mockSecretDataAdd.isDone()).toBe(true); + expect(retryMockSecretDataAdd.isDone()).toBe(true); // should call persistLocalKey twice and authenticate once expect(persistLocalKeySpy).toHaveBeenCalledTimes(2); // should call persistLocalKey twice for the first fail attempt due to invalid auth token error and the second attempt succeeds @@ -1982,6 +1984,16 @@ describe('SeedlessOnboardingController', () => { }, ]); + // do not add a duplicate backup + await baseMessenger.call( + 'SeedlessOnboardingController:addNewSecretData', + NEW_KEY_RING_1.seedPhrase, + EncAccountDataType.ImportedSrp, + { + keyringId: NEW_KEY_RING_1.id, + }, + ); + // add another seed phrase backup const mockSecretDataAdd2 = handleMockSecretDataAdd(); await baseMessenger.call( @@ -6831,7 +6843,7 @@ describe('SeedlessOnboardingController', () => { ); expect(mockRefreshJWTToken).toHaveBeenCalled(); - expect(addSecretDataItemSpy).toHaveBeenCalledTimes(1); + expect(addSecretDataItemSpy).toHaveBeenCalledTimes(2); expect(authenticateSpy).toHaveBeenCalled(); // should call persistLocalKey twice, once for the first call and another from the refresh token expect(persistLocalKeySpy).toHaveBeenCalledTimes(2); diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index ba24723ff4e..42727db4657 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -653,7 +653,7 @@ export class SeedlessOnboardingController< }); const performKeyCreationAndBackup = async (): Promise => { // encrypt and store the secret data - await this.#encryptAndStoreSecretData({ + const secretMetadata = await this.#encryptAndStoreSecretData({ data: seedPhrase, dataType: EncAccountDataType.PrimarySrp, encKey, @@ -661,6 +661,7 @@ export class SeedlessOnboardingController< options: { keyringId, }, + skipStatePersist: true, }); // store/persist the encryption key shares @@ -678,6 +679,12 @@ export class SeedlessOnboardingController< // Mark migration as complete since this new backup was created with the new data format (dataType) this.#setMigrationVersion(SeedlessOnboardingMigrationVersion.V1); + + this.#filterDupesAndUpdateSocialBackupsMetadata({ + keyringId, + data: seedPhrase, + type: secretMetadata.type, + }); }; await this.#executeWithTokenRefresh( @@ -1744,8 +1751,10 @@ export class SeedlessOnboardingController< * @param params.authKeyPair - The authentication key pair to store. * @param params.options - Optional options object. * @param params.options.keyringId - The keyring id of the backup keyring (required for SRP types). + * @param params.skipStatePersist - Whether to persist the backup state after + * the remote metadata is stored. When true, the backup metadata is not persisted to the controller state. * - * @returns A promise that resolves to the success of the operation. + * @returns A promise that resolves to the backup metadata. */ async #encryptAndStoreSecretData(params: { data: Uint8Array; @@ -1755,8 +1764,16 @@ export class SeedlessOnboardingController< options?: { keyringId?: string; }; - }): Promise { - const { options, data, encKey, authKeyPair, dataType } = params; + skipStatePersist?: boolean; + }): Promise { + const { + options, + data, + encKey, + authKeyPair, + dataType, + skipStatePersist = false, + } = params; const secretMetadata = new SecretMetadata(data, { dataType }); const { type } = secretMetadata; @@ -1764,7 +1781,7 @@ export class SeedlessOnboardingController< // before encrypting and create backup, we will check the state if the secret data is already backed up const backupState = this.getSecretDataBackupState(data, type); if (backupState) { - return; + return secretMetadata; } const secretData = secretMetadata.toBytes(); @@ -1777,7 +1794,7 @@ export class SeedlessOnboardingController< } try { - await this.#withPersistedSecretMetadataBackupsState(async () => { + const persistMetadataFn = async (): Promise & { data: Uint8Array }> => { await this.toprfClient.addSecretDataItem({ encKey, secretData, @@ -1789,7 +1806,15 @@ export class SeedlessOnboardingController< data, type, }; - }); + }; + + if (skipStatePersist) { + await persistMetadataFn(); + } else { + await this.#withPersistedSecretMetadataBackupsState(persistMetadataFn); + } + + return secretMetadata; } catch (error) { if (this.#isAuthTokenError(error)) { throw error; From 5dc8c1ae870c6dc4f1ab3336a987c91fd51116b5 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 17:57:51 +0800 Subject: [PATCH 2/7] chore: update CHANGELOG --- packages/seedless-onboarding-controller/CHANGELOG.md | 4 ++++ .../src/SeedlessOnboardingController.ts | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 37ee911dde7..2dfe586a9b6 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/utils` from `^11.12.0` to `^12.0.0` ([#10192](https://github.com/MetaMask/core/pull/10192)) +### Fixed + +- Prevent initial TOPRF seed phrase backup retries from skipping the remote metadata write after a partial failure ([#10419](https://github.com/MetaMask/core/pull/10419)) + ## [11.0.0] ### Changed diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 42727db4657..015a1191e8b 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -1794,7 +1794,9 @@ export class SeedlessOnboardingController< } try { - const persistMetadataFn = async (): Promise & { data: Uint8Array }> => { + const persistMetadataFn = async (): Promise< + Omit & { data: Uint8Array } + > => { await this.toprfClient.addSecretDataItem({ encKey, secretData, From 74fbd32113705fb955d972ad7c2a952a03e669a4 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 18:00:52 +0800 Subject: [PATCH 3/7] chore: comments --- .../src/SeedlessOnboardingController.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 015a1191e8b..d8a8bf996bd 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -661,6 +661,7 @@ export class SeedlessOnboardingController< options: { keyringId, }, + // Persist local backup state only after the full operation succeeds. skipStatePersist: true, }); @@ -680,6 +681,7 @@ export class SeedlessOnboardingController< // Mark migration as complete since this new backup was created with the new data format (dataType) this.#setMigrationVersion(SeedlessOnboardingMigrationVersion.V1); + // Persist local backup state after all operations succeed. this.#filterDupesAndUpdateSocialBackupsMetadata({ keyringId, data: seedPhrase, @@ -1810,6 +1812,7 @@ export class SeedlessOnboardingController< }; }; + // Skip local persistence if `skipStatePersist` is true. if (skipStatePersist) { await persistMetadataFn(); } else { From f2700a9002b30ec9f380d82b543a6c54507d0f5d Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 19:39:44 +0800 Subject: [PATCH 4/7] fix: reject duplicate create Toprf Key request --- .../CHANGELOG.md | 1 + .../src/SeedlessOnboardingController.test.ts | 43 +++++++++++++------ .../src/SeedlessOnboardingController.ts | 6 +++ .../src/constants.ts | 1 + 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 2dfe586a9b6..d4dd09f7909 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Prevent initial TOPRF seed phrase backup retries from skipping the remote metadata write after a partial failure ([#10419](https://github.com/MetaMask/core/pull/10419)) +- Reject duplicate TOPRF key and seed phrase backup attempts when a vault already exists ([#10419](https://github.com/MetaMask/core/pull/10419)) ## [11.0.0] diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 20f5f936b49..607a1f7dd7f 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1402,6 +1402,37 @@ describe('SeedlessOnboardingController', () => { ); }); + it('should throw if a vault already exists', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); + + await expect( + baseMessenger.call( + 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.VaultAlreadyExists, + ); + }, + ); + }); + it('should store accessToken in the vault during backup creation', async () => { await withController( { @@ -3951,7 +3982,6 @@ describe('SeedlessOnboardingController', () => { describe('changePassword', () => { const MOCK_PASSWORD = 'mock-password'; const NEW_MOCK_PASSWORD = 'new-mock-password'; - const MOCK_VAULT = JSON.stringify({ foo: 'bar' }); it('should be able to update new password', async () => { await withController( @@ -4128,7 +4158,6 @@ describe('SeedlessOnboardingController', () => { await withController( { state: getMockInitialControllerState({ - vault: MOCK_VAULT, authPubKey: MOCK_AUTH_PUB_KEY_OUTDATED, withMockAuthenticatedUser: true, }), @@ -5051,19 +5080,9 @@ describe('SeedlessOnboardingController', () => { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, - vault: 'mock-vault', }), }, async ({ controller, toprfClient, baseMessenger }) => { - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - '', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.WrongPasswordType, - ); - // Setup and store keyring encryption key. await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index d8a8bf996bd..18947189270 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -646,6 +646,12 @@ export class SeedlessOnboardingController< // assert that the user is authenticated before creating the TOPRF key and backing up the seed phrase this.#assertIsAuthenticatedUser(this.state); + if (this.state.vault) { + throw new Error( + SeedlessOnboardingControllerErrorMessage.VaultAlreadyExists, + ); + } + // locally evaluate the encryption key from the password const { encKey, pwEncKey, authKeyPair, oprfKey } = await this.toprfClient.createLocalKey({ diff --git a/packages/seedless-onboarding-controller/src/constants.ts b/packages/seedless-onboarding-controller/src/constants.ts index e89287ef7cd..5c0b6d34308 100644 --- a/packages/seedless-onboarding-controller/src/constants.ts +++ b/packages/seedless-onboarding-controller/src/constants.ts @@ -44,6 +44,7 @@ export enum SeedlessOnboardingControllerErrorMessage { InvalidVaultData = `${controllerName} - Invalid vault data`, VaultDataError = `${controllerName} - The decrypted vault has an unexpected shape.`, VaultError = `${controllerName} - Cannot unlock without a previous vault.`, + VaultAlreadyExists = `${controllerName} - Vault already exists.`, InvalidSecretMetadata = `${controllerName} - Invalid secret metadata`, MissingKeyringId = `${controllerName} - Keyring ID is required to store SRP backups.`, FailedToEncryptAndStoreSecretData = `${controllerName} - Failed to encrypt and store secret data`, From 57804def337973ece0acb2ea8b8520a28a328dda Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 19:56:46 +0800 Subject: [PATCH 5/7] chore: add tests --- .../src/SeedlessOnboardingController.test.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 607a1f7dd7f..c1f965efb4a 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1402,6 +1402,71 @@ describe('SeedlessOnboardingController', () => { ); }); + it('should retry the remote backup after vault creation fails', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, toprfClient, encryptor, baseMessenger }) => { + const { createLocalKeySpy } = mockcreateLocalKey( + toprfClient, + MOCK_PASSWORD, + ); + const retryToprfEncryptor = createMockToprfEncryptor(); + createLocalKeySpy.mockResolvedValueOnce({ + encKey: retryToprfEncryptor.deriveEncKey('retry-password'), + pwEncKey: retryToprfEncryptor.derivePwEncKey('retry-password'), + authKeyPair: + retryToprfEncryptor.deriveAuthKeyPair('retry-password'), + oprfKey: BigInt(1), + seed: stringToBytes('retry-password'), + }); + + jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValue(); + + // The first attempt writes the remote metadata, then fails while + // creating the vault. The retry must therefore write the metadata + // again using the newly generated TOPRF key. + const firstMockSecretDataAdd = handleMockSecretDataAdd(); + const retryMockSecretDataAdd = handleMockSecretDataAdd(); + jest + .spyOn(encryptor, 'encryptWithDetail') + .mockRejectedValueOnce(new Error('Failed to create vault')); + + await expect( + baseMessenger.call( + 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ), + ).rejects.toThrow('Failed to create vault'); + + expect(firstMockSecretDataAdd.isDone()).toBe(true); + expect(controller.state.socialBackupsMetadata).toStrictEqual([]); + + await baseMessenger.call( + 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); + + expect(retryMockSecretDataAdd.isDone()).toBe(true); + expect(createLocalKeySpy).toHaveBeenCalledTimes(2); + expect(controller.state.socialBackupsMetadata).toStrictEqual([ + { + type: SecretType.Mnemonic, + keyringId: MOCK_KEYRING_ID, + hash: keccak256AndHexify(MOCK_SEED_PHRASE), + }, + ]); + }, + ); + }); + it('should throw if a vault already exists', async () => { await withController( { From 74942dbd55d4b331e3b231398d8764976330d148 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 19:59:34 +0800 Subject: [PATCH 6/7] chore: add more assertions --- .../src/SeedlessOnboardingController.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index c1f965efb4a..fca0220cb59 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1424,7 +1424,7 @@ describe('SeedlessOnboardingController', () => { seed: stringToBytes('retry-password'), }); - jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValue(); + const persistLocalKeySpy = jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValue(); // The first attempt writes the remote metadata, then fails while // creating the vault. The retry must therefore write the metadata @@ -1456,6 +1456,7 @@ describe('SeedlessOnboardingController', () => { expect(retryMockSecretDataAdd.isDone()).toBe(true); expect(createLocalKeySpy).toHaveBeenCalledTimes(2); + expect(persistLocalKeySpy).toHaveBeenCalledTimes(2); expect(controller.state.socialBackupsMetadata).toStrictEqual([ { type: SecretType.Mnemonic, From 0d62b5100e7fa1733de2b2c499e03f5b742c3f35 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 24 Sep 2026 20:04:18 +0800 Subject: [PATCH 7/7] fix: lint --- .../src/SeedlessOnboardingController.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index fca0220cb59..63c3aad5c7d 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1424,7 +1424,9 @@ describe('SeedlessOnboardingController', () => { seed: stringToBytes('retry-password'), }); - const persistLocalKeySpy = jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValue(); + const persistLocalKeySpy = jest + .spyOn(toprfClient, 'persistLocalKey') + .mockResolvedValue(); // The first attempt writes the remote metadata, then fails while // creating the vault. The retry must therefore write the metadata