From 5268ccb767bedd3bd15057b27d5c02a03d5f2daf Mon Sep 17 00:00:00 2001 From: Justin Hernandez Date: Fri, 3 Apr 2026 17:27:21 -0700 Subject: [PATCH] Fix recovery rollback handling (#1905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix recovery rollback handling * Restore registration state on rollback * Restore selected document on rollback * fix(webview): clear both keys on partial rollback to prevent mnemonic/secret mismatch When restoreSnapshotBestEffort partially fails (e.g. mnemonic rollback fails but secret rollback succeeds), the stored mnemonic and private key can end up mismatched — deriving from the stored mnemonic produces a different key than what's stored. This is silent data corruption that could lock users out of recovery. Fix: when any rollback write fails, clear both keys so ensureSecret can regenerate a consistent pair from scratch. A missing pair is recoverable; a mismatched pair is not. Adds a test in restoreSecretFromMnemonic that proves the mismatch scenario and verifies both keys are cleared. * feat(new-common): add humanizeContractError utility with tests * fix: prettier formatting in secretManager test --------- Co-authored-by: Tranquil-Flow --- .../src/blockchain/contractErrors.test.ts | 65 ++++++ new-common/src/blockchain/contractErrors.ts | 71 ++++++ .../src/proving/recoveryValidation.ts | 63 +++++- .../tests/proving/recoveryValidation.test.ts | 193 +++++++++++++++- packages/webview-app/package.json | 2 + .../recovery/SecretPhraseInputScreen.tsx | 38 +++- .../src/screens/tunnel/TourScreen.tsx | 29 +-- .../webview-app/src/utils/secretManager.ts | 91 ++++++-- .../recovery/secretPhraseInputScreen.test.tsx | 212 ++++++++++++++++++ .../tests/utils/secretManager.test.ts | 170 +++++++++++++- 10 files changed, 884 insertions(+), 50 deletions(-) create mode 100644 new-common/src/blockchain/contractErrors.test.ts create mode 100644 new-common/src/blockchain/contractErrors.ts create mode 100644 packages/webview-app/tests/screens/recovery/secretPhraseInputScreen.test.tsx diff --git a/new-common/src/blockchain/contractErrors.test.ts b/new-common/src/blockchain/contractErrors.test.ts new file mode 100644 index 000000000..2cf9b6d6a --- /dev/null +++ b/new-common/src/blockchain/contractErrors.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest'; + +import { humanizeContractError } from './contractErrors.js'; + +describe('humanizeContractError', () => { + it('decodes a known custom error selector', () => { + // 0xda7bd3a6 = InvalidVcAndDiscloseProof + expect(humanizeContractError('0xda7bd3a6')).toBe('Invalid Vc And Disclose Proof'); + }); + + it('decodes a known SCREAMING_CASE selector', () => { + // 0x034acfcc = REGISTERED_COMMITMENT + expect(humanizeContractError('0x034acfcc')).toBe('Registered Commitment'); + }); + + it('decodes Error(string) standard revert', () => { + // ABI encoding of Error("Insufficient balance") + const encoded = + '0x08c379a0' + + '0000000000000000000000000000000000000000000000000000000000000020' + + '0000000000000000000000000000000000000000000000000000000000000014' + + '496e73756666696369656e742062616c616e636500000000000000000000000000'; + expect(humanizeContractError(encoded)).toBe('Insufficient balance'); + }); + + it('decodes Panic(uint256) arithmetic overflow', () => { + const encoded = + '0x4e487b71' + '0000000000000000000000000000000000000000000000000000000000000011'; + expect(humanizeContractError(encoded)).toBe('Arithmetic overflow or underflow'); + }); + + it('decodes Panic(uint256) division by zero', () => { + const encoded = + '0x4e487b71' + '0000000000000000000000000000000000000000000000000000000000000012'; + expect(humanizeContractError(encoded)).toBe('Division or modulo by zero'); + }); + + it('decodes Panic(uint256) array out of bounds', () => { + const encoded = + '0x4e487b71' + '0000000000000000000000000000000000000000000000000000000000000032'; + expect(humanizeContractError(encoded)).toBe('Array out of bounds'); + }); + + it('decodes Panic(uint256) unknown code gracefully', () => { + const encoded = + '0x4e487b71' + '00000000000000000000000000000000000000000000000000000000000000ff'; + expect(humanizeContractError(encoded)).toBe('Contract panic (code 255)'); + }); + + it('returns original string for unknown selector', () => { + expect(humanizeContractError('0xdeadbeef')).toBe('0xdeadbeef'); + }); + + it('returns original string for non-hex input', () => { + expect(humanizeContractError('something went wrong')).toBe('something went wrong'); + }); + + it('returns original string for empty input', () => { + expect(humanizeContractError('')).toBe(''); + }); + + it('handles mixed-case selector input', () => { + expect(humanizeContractError('0xDA7BD3A6')).toBe('Invalid Vc And Disclose Proof'); + }); +}); diff --git a/new-common/src/blockchain/contractErrors.ts b/new-common/src/blockchain/contractErrors.ts new file mode 100644 index 000000000..acb2b2a48 --- /dev/null +++ b/new-common/src/blockchain/contractErrors.ts @@ -0,0 +1,71 @@ +import { AbiCoder } from 'ethers'; + +import selectorMap from '../data/error-selector-map.json' with { type: 'json' }; + +const SELECTOR_RE = /^0x[0-9a-fA-F]{8}/; +const ERROR_STRING_SELECTOR = '0x08c379a0'; +const PANIC_SELECTOR = '0x4e487b71'; + +const PANIC_CODES: Record = { + 0x01: 'Assertion failed', + 0x11: 'Arithmetic overflow or underflow', + 0x12: 'Division or modulo by zero', + 0x21: 'Invalid enum value', + 0x22: 'Corrupted storage byte array', + 0x31: 'Pop on empty array', + 0x32: 'Array out of bounds', + 0x41: 'Out of memory', + 0x51: 'Invalid internal function call', +}; + +function formatErrorName(name: string): string { + if (name === name.toUpperCase() && name.includes('_')) { + return name + .split('_') + .map(word => word.charAt(0) + word.slice(1).toLowerCase()) + .join(' '); + } + return name.replace(/([a-z])([A-Z])/g, '$1 $2').replace(/([A-Z]+)([A-Z][a-z])/g, '$1 $2'); +} + +/** + * Converts a raw Solidity error string into a human-readable message. + * + * Handles: + * - Standard Error(string): extracts the revert message + * - Standard Panic(uint256): maps panic code to description + * - Known custom error selectors from our contracts (auto-generated map) + * - Unknown input: returned unchanged + */ +export function humanizeContractError(raw: string): string { + if (!raw) return raw; + + const lower = raw.toLowerCase(); + + if (lower.startsWith(ERROR_STRING_SELECTOR)) { + try { + const [message] = AbiCoder.defaultAbiCoder().decode(['string'], '0x' + raw.slice(10)); + return message as string; + } catch { + return raw; + } + } + + if (lower.startsWith(PANIC_SELECTOR)) { + try { + const [code] = AbiCoder.defaultAbiCoder().decode(['uint256'], '0x' + raw.slice(10)); + const codeNum = Number(code); + return PANIC_CODES[codeNum] ?? `Contract panic (code ${codeNum})`; + } catch { + return raw; + } + } + + if (SELECTOR_RE.test(raw)) { + const selector = lower.slice(0, 10); + const name = (selectorMap as Record)[selector]; + if (name) return formatErrorName(name); + } + + return raw; +} diff --git a/packages/mobile-sdk-alpha/src/proving/recoveryValidation.ts b/packages/mobile-sdk-alpha/src/proving/recoveryValidation.ts index 190828a6b..a2c3b2a93 100644 --- a/packages/mobile-sdk-alpha/src/proving/recoveryValidation.ts +++ b/packages/mobile-sdk-alpha/src/proving/recoveryValidation.ts @@ -5,7 +5,13 @@ import type { DocumentCategory, IDDocument } from '@selfxyz/common'; import { isUserRegisteredWithAlternativeCSCA } from '@selfxyz/common/utils/passports/validate'; -import { markCurrentDocumentAsRegistered, reStorePassportDataWithRightCSCA } from '../documents/utils'; +import { cloneForStorage } from '../adapters/browser/documents'; +import { + markCurrentDocumentAsRegistered, + reStorePassportDataWithRightCSCA, + storePassportData, + updateDocumentRegistrationState, +} from '../documents/utils'; import { getCommitmentTree } from '../stores'; import type { SelfClient } from '../types/public'; @@ -28,11 +34,58 @@ export async function finalizeRecoveredDocumentRegistration( document: IDDocument, csca?: string, ): Promise { - if (csca) { - await reStorePassportDataWithRightCSCA(selfClient, document, csca); - } + const originalDocument = cloneForStorage(document); + const originalCatalog = await selfClient.loadDocumentCatalog(); + const originalSelectedDocument = originalCatalog.selectedDocumentId + ? originalCatalog.documents.find(doc => doc.id === originalCatalog.selectedDocumentId) + : undefined; + const originalSelectedDocumentSnapshot = originalSelectedDocument + ? { + id: originalSelectedDocument.id, + isRegistered: originalSelectedDocument.isRegistered, + registeredAt: originalSelectedDocument.registeredAt, + } + : undefined; - await markCurrentDocumentAsRegistered(selfClient); + try { + if (csca) { + await reStorePassportDataWithRightCSCA(selfClient, document, csca); + } + + await markCurrentDocumentAsRegistered(selfClient); + } catch (error) { + if (csca) { + try { + await storePassportData(selfClient, originalDocument); + } catch (rollbackError) { + console.error('Rollback failed while restoring the original document during recovery:', rollbackError); + } + } + + if (originalSelectedDocumentSnapshot) { + try { + const rollbackCatalog = await selfClient.loadDocumentCatalog(); + rollbackCatalog.selectedDocumentId = originalCatalog.selectedDocumentId; + const rollbackDocument = rollbackCatalog.documents.find(doc => doc.id === originalSelectedDocumentSnapshot.id); + + if (rollbackDocument) { + rollbackDocument.isRegistered = originalSelectedDocumentSnapshot.isRegistered; + rollbackDocument.registeredAt = originalSelectedDocumentSnapshot.registeredAt; + await selfClient.saveDocumentCatalog(rollbackCatalog); + } else { + await updateDocumentRegistrationState( + selfClient, + originalSelectedDocumentSnapshot.id, + Boolean(originalSelectedDocumentSnapshot.isRegistered), + ); + } + } catch (rollbackError) { + console.error('Rollback failed while restoring the registration flag during recovery:', rollbackError); + } + } + + throw error; + } } export async function validateRecoverySecretForDocument( diff --git a/packages/mobile-sdk-alpha/tests/proving/recoveryValidation.test.ts b/packages/mobile-sdk-alpha/tests/proving/recoveryValidation.test.ts index b3604cc91..c8e3453d1 100644 --- a/packages/mobile-sdk-alpha/tests/proving/recoveryValidation.test.ts +++ b/packages/mobile-sdk-alpha/tests/proving/recoveryValidation.test.ts @@ -6,22 +6,53 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { IDDocument } from '@selfxyz/common'; -import { validateRecoverySecretForDocument } from '../../src/proving/recoveryValidation'; +import { + finalizeRecoveredDocumentRegistration, + validateRecoverySecretForDocument, +} from '../../src/proving/recoveryValidation'; import type { SelfClient } from '../../src/types/public'; const isUserRegisteredWithAlternativeCSCAMock = vi.fn(); +const markCurrentDocumentAsRegisteredMock = vi.fn(); +const restorePassportDataWithRightCSCAmock = vi.fn(); +const storePassportDataMock = vi.fn(); +const updateDocumentRegistrationStateMock = vi.fn(); +const saveDocumentCatalogMock = vi.fn(); vi.mock('@selfxyz/common/utils/passports/validate', () => ({ isUserRegisteredWithAlternativeCSCA: (...args: unknown[]) => isUserRegisteredWithAlternativeCSCAMock(...args), })); +vi.mock('../../src/documents/utils', () => ({ + markCurrentDocumentAsRegistered: (...args: unknown[]) => markCurrentDocumentAsRegisteredMock(...args), + reStorePassportDataWithRightCSCA: (...args: unknown[]) => restorePassportDataWithRightCSCAmock(...args), + storePassportData: (...args: unknown[]) => storePassportDataMock(...args), + updateDocumentRegistrationState: (...args: unknown[]) => updateDocumentRegistrationStateMock(...args), +})); + const documentFixture = { documentCategory: 'passport', documentType: 'passport', } as IDDocument; -function createSelfClient() { +function createSelfClient({ + selectedDocumentId = 'doc-1', + documents = [], +}: { + selectedDocumentId?: string | undefined; + documents?: Array>; +} = {}) { + const catalog = { + selectedDocumentId, + documents: [...documents], + }; + return { + loadDocumentCatalog: async () => ({ + ...catalog, + documents: catalog.documents.map(doc => ({ ...doc })), + }), + saveDocumentCatalog: saveDocumentCatalogMock, getProtocolState: () => ({ passport: { commitment_tree: 'passport-tree', @@ -111,3 +142,161 @@ describe('validateRecoverySecretForDocument', () => { expect(isUserRegisteredWithAlternativeCSCAMock).toHaveBeenCalledTimes(1); }); }); + +describe('finalizeRecoveredDocumentRegistration', () => { + beforeEach(() => { + vi.clearAllMocks(); + saveDocumentCatalogMock.mockReset(); + }); + + it('rolls back document state when registration marking fails after a CSCA restore', async () => { + restorePassportDataWithRightCSCAmock.mockImplementation(async (_client, document) => { + document.passportMetadata = { + ...(document.passportMetadata ?? {}), + csca: 'new-csca', + }; + }); + markCurrentDocumentAsRegisteredMock.mockRejectedValue(new Error('catalog save failed')); + + const document = { + ...documentFixture, + passportMetadata: { + csca: 'old-csca', + }, + } as IDDocument; + + await expect( + finalizeRecoveredDocumentRegistration( + createSelfClient({ + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }), + document, + 'new-csca', + ), + ).rejects.toThrow('catalog save failed'); + + expect(restorePassportDataWithRightCSCAmock).toHaveBeenCalledWith(expect.anything(), document, 'new-csca'); + expect(storePassportDataMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + passportMetadata: expect.objectContaining({ + csca: 'old-csca', + }), + }), + ); + expect(saveDocumentCatalogMock).toHaveBeenCalledWith({ + selectedDocumentId: 'doc-1', + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }); + expect(updateDocumentRegistrationStateMock).not.toHaveBeenCalled(); + }); + + it('does not attempt document restore when mark fails without a CSCA override', async () => { + markCurrentDocumentAsRegisteredMock.mockRejectedValue(new Error('catalog save failed')); + + await expect( + finalizeRecoveredDocumentRegistration( + createSelfClient({ + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }), + documentFixture, + ), + ).rejects.toThrow('catalog save failed'); + + expect(restorePassportDataWithRightCSCAmock).not.toHaveBeenCalled(); + expect(storePassportDataMock).not.toHaveBeenCalled(); + expect(saveDocumentCatalogMock).toHaveBeenCalledWith({ + selectedDocumentId: 'doc-1', + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }); + expect(updateDocumentRegistrationStateMock).not.toHaveBeenCalled(); + }); + + it('restores the original registration state when mark fails for a previously registered document', async () => { + markCurrentDocumentAsRegisteredMock.mockRejectedValue(new Error('catalog save failed')); + + await expect( + finalizeRecoveredDocumentRegistration( + createSelfClient({ + documents: [ + { + id: 'doc-1', + isRegistered: true, + registeredAt: 1234, + }, + ], + }), + documentFixture, + ), + ).rejects.toThrow('catalog save failed'); + + expect(saveDocumentCatalogMock).toHaveBeenCalledWith({ + selectedDocumentId: 'doc-1', + documents: [ + { + id: 'doc-1', + isRegistered: true, + registeredAt: 1234, + }, + ], + }); + expect(updateDocumentRegistrationStateMock).not.toHaveBeenCalled(); + }); + + it('still restores the registration state when restoring the original document fails', async () => { + storePassportDataMock.mockRejectedValue(new Error('document rollback failed')); + markCurrentDocumentAsRegisteredMock.mockRejectedValue(new Error('catalog save failed')); + + await expect( + finalizeRecoveredDocumentRegistration( + createSelfClient({ + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }), + documentFixture, + 'new-csca', + ), + ).rejects.toThrow('catalog save failed'); + + expect(storePassportDataMock).toHaveBeenCalledTimes(1); + expect(saveDocumentCatalogMock).toHaveBeenCalledWith({ + selectedDocumentId: 'doc-1', + documents: [ + { + id: 'doc-1', + isRegistered: false, + registeredAt: undefined, + }, + ], + }); + }); +}); diff --git a/packages/webview-app/package.json b/packages/webview-app/package.json index b81b1bb0a..df84f3fa0 100644 --- a/packages/webview-app/package.json +++ b/packages/webview-app/package.json @@ -13,6 +13,8 @@ "lint:fix": "eslint . --fix --max-warnings=0", "nice": "prettier --write . && eslint . --fix && tsc --noEmit", "preview": "vite preview", + "test": "vitest run", + "test:watch": "vitest", "typecheck": "tsc --noEmit", "types": "tsc --noEmit" }, diff --git a/packages/webview-app/src/screens/recovery/SecretPhraseInputScreen.tsx b/packages/webview-app/src/screens/recovery/SecretPhraseInputScreen.tsx index b4368e038..c1b6e0f06 100644 --- a/packages/webview-app/src/screens/recovery/SecretPhraseInputScreen.tsx +++ b/packages/webview-app/src/screens/recovery/SecretPhraseInputScreen.tsx @@ -47,7 +47,13 @@ const EMPTY_WORDS = Array.from({ length: WORD_COUNT }).fill(''); const instruction = 'Enter your recovery phrase to restore your account, registered IDs, and activity history'; class RecoveryFlowError extends Error { - constructor(readonly reason: 'storage_write_failed' | 'document_finalization_failed' | 'unexpected_error') { + constructor( + readonly reason: + | 'document_unavailable' + | 'storage_write_failed' + | 'document_finalization_failed' + | 'unexpected_error', + ) { super(reason); } } @@ -180,7 +186,11 @@ export const SecretPhraseInputScreen: React.FC = () => { } const selectedDocument = await loadSelectedDocument(client); - const hasRealDocument = selectedDocument && !selectedDocument.metadata.mock; + if (selectedDocument === null) { + throw new RecoveryFlowError('document_unavailable'); + } + + const hasRealDocument = !selectedDocument.metadata.mock; if (!hasRealDocument) { await restoreSecretFromMnemonic(storage, mnemonic); @@ -193,7 +203,9 @@ export const SecretPhraseInputScreen: React.FC = () => { haptic.trigger('success'); analytics.trackEvent('recovery_phrase_recovered', { documentCategory: 'none' }); - navigate('/tunnel/kyc', { replace: true }); + if (isMountedRef.current) { + navigate('/tunnel/kyc', { replace: true }); + } return; } else { derivedSecret = derivePrivateKey(mnemonic); @@ -250,10 +262,12 @@ export const SecretPhraseInputScreen: React.FC = () => { analytics.trackEvent('recovery_phrase_recovered', { documentCategory: selectedDocument.data.documentCategory, }); - if (returnTo) { - navigate(returnTo, { replace: true }); - } else { - navigate('/recovery/success'); + if (isMountedRef.current) { + if (returnTo) { + navigate(returnTo, { replace: true }); + } else { + navigate('/recovery/success'); + } } } catch (error) { const reason = error instanceof RecoveryFlowError ? error.reason : 'unexpected_error'; @@ -262,10 +276,12 @@ export const SecretPhraseInputScreen: React.FC = () => { analytics.trackEvent('recovery_phrase_failed', { reason, }); - navigate(buildRecoveryTarget('/recovery/failure', returnTo), { - replace: true, - state: returnTo ? { returnTo } : undefined, - }); + if (isMountedRef.current) { + navigate(buildRecoveryTarget('/recovery/failure', returnTo), { + replace: true, + state: returnTo ? { returnTo } : undefined, + }); + } } finally { derivedSecret = null; if (isMountedRef.current) { diff --git a/packages/webview-app/src/screens/tunnel/TourScreen.tsx b/packages/webview-app/src/screens/tunnel/TourScreen.tsx index 46f10ac68..e73ae321a 100644 --- a/packages/webview-app/src/screens/tunnel/TourScreen.tsx +++ b/packages/webview-app/src/screens/tunnel/TourScreen.tsx @@ -24,31 +24,32 @@ export const TourScreen: React.FC = () => { return; } - const selectedDoc = await loadSelectedDocument(client); - - console.log('selected Doc', selectedDoc); - const isRegisteredRealDoc = selectedDoc?.metadata?.isRegistered === true; - - if (isRegisteredRealDoc) { - navigate('/tunnel/proof/disclose'); - } else { - navigate('/tunnel/kyc'); + try { + const selectedDoc = await loadSelectedDocument(client); + if (selectedDoc?.metadata?.isRegistered === true) { + navigate('/tunnel/proof/disclose'); + return; + } + } catch { + // Fall through to KYC when document state is unavailable. } + + navigate('/tunnel/kyc'); }, [navigate, stepNum, client]); - const onResore = useCallback(() => { + const onRestore = useCallback(() => { navigate('/recovery'); }, []); switch (step) { case '1': - return ; + return ; case '2': - return ; + return ; case '3': - return ; + return ; case '4': - return ; + return ; default: return ; } diff --git a/packages/webview-app/src/utils/secretManager.ts b/packages/webview-app/src/utils/secretManager.ts index 336cc3a20..3c1bb23d1 100644 --- a/packages/webview-app/src/utils/secretManager.ts +++ b/packages/webview-app/src/utils/secretManager.ts @@ -44,6 +44,15 @@ function withSecretLock(fn: () => Promise): Promise { return next; } +async function readStoredSecretSnapshotUnlocked(storage: BridgeStorageAdapter): Promise { + const [mnemonic, secret] = await Promise.all([storage.get(MNEMONIC_KEY), storage.get(PRIVATE_KEY_KEY)]); + + return { + mnemonic, + secret, + }; +} + export function ensureSecret(storage: BridgeStorageAdapter): Promise { return withSecretLock(async () => { const existing = await storage.get(PRIVATE_KEY_KEY); @@ -72,12 +81,7 @@ export function ensureSecret(storage: BridgeStorageAdapter): Promise { } export async function readStoredSecretSnapshot(storage: BridgeStorageAdapter): Promise { - const [mnemonic, secret] = await Promise.all([storage.get(MNEMONIC_KEY), storage.get(PRIVATE_KEY_KEY)]); - - return { - mnemonic, - secret, - }; + return withSecretLock(() => readStoredSecretSnapshotUnlocked(storage)); } export function restoreSecretFromMnemonic( @@ -87,13 +91,13 @@ export function restoreSecretFromMnemonic( const secret = derivePrivateKey(mnemonic); return withSecretLock(async () => { - const previousSnapshot = await readStoredSecretSnapshot(storage); + const previousSnapshot = await readStoredSecretSnapshotUnlocked(storage); try { await storage.set(MNEMONIC_KEY, mnemonic); await storage.set(PRIVATE_KEY_KEY, secret); } catch (error) { - await writeSnapshot(storage, previousSnapshot); + await restoreSnapshotBestEffort(storage, previousSnapshot, 'secret restore from mnemonic'); throw error; } @@ -105,19 +109,72 @@ export function restoreStoredSecretSnapshot( storage: BridgeStorageAdapter, snapshot: StoredSecretSnapshot, ): Promise { - return withSecretLock(() => writeSnapshot(storage, snapshot)); + return withSecretLock(async () => { + const previousSnapshot = await readStoredSecretSnapshotUnlocked(storage); + + try { + await writeSnapshot(storage, snapshot); + } catch (error) { + await restoreSnapshotBestEffort(storage, previousSnapshot, 'secret snapshot restore'); + throw error; + } + }); +} + +async function restoreSnapshotBestEffort( + storage: BridgeStorageAdapter, + snapshot: StoredSecretSnapshot, + context: string, +): Promise { + const rollbackFailures: Error[] = []; + + try { + await writeMnemonic(storage, snapshot.mnemonic); + } catch (rollbackError) { + rollbackFailures.push(rollbackError instanceof Error ? rollbackError : new Error(String(rollbackError))); + } + + try { + await writeSecret(storage, snapshot.secret); + } catch (rollbackError) { + rollbackFailures.push(rollbackError instanceof Error ? rollbackError : new Error(String(rollbackError))); + } + + if (rollbackFailures.length > 0) { + // A partial rollback leaves mnemonic and secret mismatched — the derived + // key from the stored mnemonic would not equal the stored secret. Clear + // both so ensureSecret can regenerate a consistent pair from scratch. + console.error(`Rollback failed during ${context}, clearing both keys to prevent mismatch:`, rollbackFailures); + try { + await storage.remove(MNEMONIC_KEY); + } catch { + /* best effort */ + } + try { + await storage.remove(PRIVATE_KEY_KEY); + } catch { + /* best effort */ + } + } } async function writeSnapshot(storage: BridgeStorageAdapter, snapshot: StoredSecretSnapshot): Promise { - if (snapshot.mnemonic === null) { + await writeMnemonic(storage, snapshot.mnemonic); + await writeSecret(storage, snapshot.secret); +} + +async function writeMnemonic(storage: BridgeStorageAdapter, mnemonic: string | null): Promise { + if (mnemonic === null) { await storage.remove(MNEMONIC_KEY); } else { - await storage.set(MNEMONIC_KEY, snapshot.mnemonic); - } - - if (snapshot.secret === null) { - await storage.remove(PRIVATE_KEY_KEY); - } else { - await storage.set(PRIVATE_KEY_KEY, snapshot.secret); + await storage.set(MNEMONIC_KEY, mnemonic); + } +} + +async function writeSecret(storage: BridgeStorageAdapter, secret: string | null): Promise { + if (secret === null) { + await storage.remove(PRIVATE_KEY_KEY); + } else { + await storage.set(PRIVATE_KEY_KEY, secret); } } diff --git a/packages/webview-app/tests/screens/recovery/secretPhraseInputScreen.test.tsx b/packages/webview-app/tests/screens/recovery/secretPhraseInputScreen.test.tsx new file mode 100644 index 000000000..4b69a3966 --- /dev/null +++ b/packages/webview-app/tests/screens/recovery/secretPhraseInputScreen.test.tsx @@ -0,0 +1,212 @@ +// SPDX-FileCopyrightText: 2025-2026 Social Connect Labs, Inc. +// SPDX-License-Identifier: BUSL-1.1 +// NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. + +// @vitest-environment jsdom + +import type React from 'react'; +import { useEffect } from 'react'; +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { SecretPhraseInputScreen } from '../../../src/screens/recovery/SecretPhraseInputScreen'; + +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; + +const analytics = { trackEvent: vi.fn() }; +const haptic = { trigger: vi.fn() }; +const lifecycle = { dismiss: vi.fn(), setResult: vi.fn() }; +const client = { id: 'client' }; + +const loadSelectedDocumentMock = vi.fn(); +const validateRecoverySecretForDocumentMock = vi.fn(); +const finalizeRecoveredDocumentRegistrationMock = vi.fn(); +const restoreSecretFromMnemonicMock = vi.fn(); +const readStoredSecretSnapshotMock = vi.fn(); +const restoreStoredSecretSnapshotMock = vi.fn(); + +vi.mock('../../../src/providers/SelfClientProvider', () => ({ + useSelfClient: () => ({ + client, + analytics, + haptic, + lifecycle, + }), +})); + +vi.mock('../../../src/providers/BridgeProvider', () => ({ + useBridge: () => ({}), +})); + +vi.mock('@selfxyz/webview-bridge/adapters', () => ({ + bridgeStorageAdapter: () => ({ + get: vi.fn().mockResolvedValue(null), + set: vi.fn().mockResolvedValue(undefined), + remove: vi.fn().mockResolvedValue(undefined), + }), +})); + +vi.mock('@selfxyz/mobile-sdk-alpha/browser', () => ({ + loadSelectedDocument: (...args: unknown[]) => loadSelectedDocumentMock(...args), + validateRecoverySecretForDocument: (...args: unknown[]) => validateRecoverySecretForDocumentMock(...args), + finalizeRecoveredDocumentRegistration: (...args: unknown[]) => finalizeRecoveredDocumentRegistrationMock(...args), +})); + +vi.mock('../../../src/utils/secretManager', () => ({ + derivePrivateKey: (mnemonic: string) => `derived-from-${mnemonic.split(' ')[0]}`, + readStoredSecretSnapshot: (...args: unknown[]) => readStoredSecretSnapshotMock(...args), + restoreSecretFromMnemonic: (...args: unknown[]) => restoreSecretFromMnemonicMock(...args), + restoreStoredSecretSnapshot: (...args: unknown[]) => restoreStoredSecretSnapshotMock(...args), +})); + +vi.mock('../../../src/utils/insets', () => ({ + WEB_SAFE_AREA: { + insets: { top: 0, bottom: 0, left: 0, right: 0 }, + safeArea: { top: 0, bottom: 0, left: 0, right: 0 }, + }, +})); + +const VALID_12_MNEMONIC = + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'; + +vi.mock('@selfxyz/euclid', () => ({ + Button: ({ text, onPress, disabled }: { text: string; onPress: () => void; disabled?: boolean }) => ( + + ), + colors: { slate50: '#f8fafc', black: '#000', white: '#fff', red600: '#dc2626', slate300: '#cbd5e1' }, + fontFamily: { dinOT: 'DIN OT' }, + fontWeight: { medium: 500 }, + spacing: { md: 12, mdLg: 16, lg: 20, xl: 24, xlLg: 32, sm: 8 }, + LeftArrowIcon: () => null, + SecretPhraseInput: ({ onWordChange }: { onWordChange: (index: number, word: string) => void }) => { + useEffect(() => { + VALID_12_MNEMONIC.split(' ').forEach((w, i) => onWordChange(i, w)); + }, []); + return
; + }, + TopNavigationDialogue: () =>
, +})); + +const LocationDisplay: React.FC = () => { + const location = useLocation(); + return
{`${location.pathname}${location.search}`}
; +}; + +const renderScreen = (initialEntry = '/recovery/phrase-input') => + render( + + + } /> + } /> + } /> + } /> + } /> + + + , + ); + +async function fillWordsAndSubmit() { + await act(async () => { + await new Promise(r => setTimeout(r, 0)); + }); + fireEvent.click(screen.getByRole('button', { name: /continue/i })); +} + +describe('SecretPhraseInputScreen', () => { + beforeEach(() => { + vi.clearAllMocks(); + restoreSecretFromMnemonicMock.mockResolvedValue({ secret: 'derived-secret' }); + readStoredSecretSnapshotMock.mockResolvedValue({ mnemonic: null, secret: null }); + restoreStoredSecretSnapshotMock.mockResolvedValue(undefined); + }); + + afterEach(() => { + cleanup(); + }); + + const expectLocation = (expected: string) => { + const locations = screen.getAllByTestId('location'); + expect(locations.at(-1)?.textContent).toBe(expected); + }; + + it('navigates to failure when loadSelectedDocument returns null', async () => { + loadSelectedDocumentMock.mockResolvedValue(null); + + renderScreen(); + await fillWordsAndSubmit(); + + await waitFor(() => { + expectLocation('/recovery/failure'); + }); + + expect(analytics.trackEvent).toHaveBeenCalledWith('recovery_phrase_failed', { + reason: 'document_unavailable', + }); + expect(haptic.trigger).toHaveBeenCalledWith('error'); + }); + + it('navigates to /tunnel/kyc for a mock document', async () => { + loadSelectedDocumentMock.mockResolvedValue({ + data: { documentCategory: 'passport' }, + metadata: { mock: true }, + }); + + renderScreen(); + await fillWordsAndSubmit(); + + await waitFor(() => { + expectLocation('/tunnel/kyc'); + }); + + expect(restoreSecretFromMnemonicMock).toHaveBeenCalled(); + }); + + it('does not navigate after unmount during async validation', async () => { + let resolveDocument!: (value: unknown) => void; + loadSelectedDocumentMock.mockImplementation( + () => + new Promise(resolve => { + resolveDocument = resolve; + }), + ); + + const { unmount } = renderScreen(); + await fillWordsAndSubmit(); + + unmount(); + + await act(async () => { + resolveDocument(null); + await new Promise(r => setTimeout(r, 0)); + }); + + expect(haptic.trigger).toHaveBeenCalledWith('error'); + }); + + it('navigates to failure when storage write fails during recovery', async () => { + loadSelectedDocumentMock.mockResolvedValue({ + data: { documentCategory: 'passport' }, + metadata: { mock: false, isRegistered: true }, + }); + validateRecoverySecretForDocumentMock.mockResolvedValue({ + isRegistered: true, + csca: 'matching-csca', + }); + restoreSecretFromMnemonicMock.mockRejectedValue(new Error('write failed')); + + renderScreen(); + await fillWordsAndSubmit(); + + await waitFor(() => { + expectLocation('/recovery/failure'); + }); + + expect(analytics.trackEvent).toHaveBeenCalledWith('recovery_phrase_failed', { + reason: 'storage_write_failed', + }); + expect(restoreStoredSecretSnapshotMock).toHaveBeenCalled(); + }); +}); diff --git a/packages/webview-app/tests/utils/secretManager.test.ts b/packages/webview-app/tests/utils/secretManager.test.ts index 75df944e8..bc2b1ba97 100644 --- a/packages/webview-app/tests/utils/secretManager.test.ts +++ b/packages/webview-app/tests/utils/secretManager.test.ts @@ -6,7 +6,13 @@ import { beforeEach, describe, expect, it } from 'vitest'; -import { derivePrivateKey, ensureSecret, restoreSecretFromMnemonic } from '../../src/utils/secretManager'; +import { + derivePrivateKey, + ensureSecret, + readStoredSecretSnapshot, + restoreSecretFromMnemonic, + restoreStoredSecretSnapshot, +} from '../../src/utils/secretManager'; type Deferred = { promise: Promise; @@ -105,6 +111,43 @@ describe('restoreSecretFromMnemonic', () => { expect(storageState.get('self_mnemonic')).toBe(mnemonic); expect(storageState.get('self_private_key')).toBe(derivePrivateKey(mnemonic)); }); + + it('clears both keys when mnemonic rollback fails to prevent mismatch', async () => { + const storageState = new Map(); + const originalMnemonic = 'legal winner thank year wave sausage worth useful legal winner thank yellow'; + const originalSecret = derivePrivateKey(originalMnemonic); + + storageState.set('self_mnemonic', originalMnemonic); + storageState.set('self_private_key', originalSecret); + + const newMnemonic = 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'; + const newSecret = derivePrivateKey(newMnemonic); + + let failPrivateKeyWrite = true; + let failMnemonicRollback = true; + const storage = { + get: async (key: string) => storageState.get(key) ?? null, + set: async (key: string, value: string) => { + if (key === 'self_private_key' && value === newSecret && failPrivateKeyWrite) { + failPrivateKeyWrite = false; + throw new Error('private key write failed'); + } + if (key === 'self_mnemonic' && value === originalMnemonic && failMnemonicRollback) { + failMnemonicRollback = false; + throw new Error('mnemonic rollback failed'); + } + storageState.set(key, value); + }, + remove: async (key: string) => { + storageState.delete(key); + }, + }; + + await expect(restoreSecretFromMnemonic(storage, newMnemonic)).rejects.toThrow('private key write failed'); + + expect(storageState.get('self_mnemonic')).toBeUndefined(); + expect(storageState.get('self_private_key')).toBeUndefined(); + }); }); describe('ensureSecret', () => { @@ -154,3 +197,128 @@ describe('ensureSecret', () => { expect(storageState.get('self_private_key')).toBe('0xexisting'); }); }); + +describe('readStoredSecretSnapshot', () => { + it('reads mnemonic and private key under the shared lock', async () => { + const storageState = new Map(); + const firstMnemonicWrite = createDeferred(); + let mnemonicSetCount = 0; + + storageState.set( + 'self_mnemonic', + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + ); + storageState.set('self_private_key', derivePrivateKey(storageState.get('self_mnemonic')!)); + + const storage = { + get: async (key: string) => storageState.get(key) ?? null, + set: async (key: string, value: string) => { + if (key === 'self_mnemonic') { + mnemonicSetCount += 1; + if (mnemonicSetCount === 1) { + storageState.set(key, value); + await firstMnemonicWrite.promise; + return; + } + } + + storageState.set(key, value); + }, + remove: async (key: string) => { + storageState.delete(key); + }, + }; + + const nextMnemonic = 'legal winner thank year wave sausage worth useful legal winner thank yellow'; + const restorePromise = restoreSecretFromMnemonic(storage, nextMnemonic); + const snapshotPromise = readStoredSecretSnapshot(storage); + + await Promise.resolve(); + firstMnemonicWrite.resolve(); + + await restorePromise; + const snapshot = await snapshotPromise; + + expect(snapshot).toEqual({ + mnemonic: nextMnemonic, + secret: derivePrivateKey(nextMnemonic), + }); + }); +}); + +describe('restoreStoredSecretSnapshot', () => { + it('restores the previous snapshot when writing the replacement snapshot fails', async () => { + const storageState = new Map(); + const targetSnapshot = { + mnemonic: 'legal winner thank year wave sausage worth useful legal winner thank yellow', + secret: derivePrivateKey('legal winner thank year wave sausage worth useful legal winner thank yellow'), + }; + + storageState.set( + 'self_mnemonic', + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + ); + storageState.set('self_private_key', derivePrivateKey(storageState.get('self_mnemonic')!)); + + let failPrivateKeyWrite = true; + const storage = { + get: async (key: string) => storageState.get(key) ?? null, + set: async (key: string, value: string) => { + storageState.set(key, value); + if (key === 'self_private_key' && failPrivateKeyWrite) { + failPrivateKeyWrite = false; + throw new Error('write failed'); + } + }, + remove: async (key: string) => { + storageState.delete(key); + }, + }; + + await expect(restoreStoredSecretSnapshot(storage, targetSnapshot)).rejects.toThrow('write failed'); + expect(storageState.get('self_mnemonic')).toBe( + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + ); + expect(storageState.get('self_private_key')).toBe( + derivePrivateKey('abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'), + ); + }); + + it('clears both keys when mnemonic rollback fails to prevent mismatch', async () => { + const storageState = new Map(); + const originalMnemonic = + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'; + const originalSecret = derivePrivateKey(originalMnemonic); + const targetSnapshot = { + mnemonic: 'legal winner thank year wave sausage worth useful legal winner thank yellow', + secret: derivePrivateKey('legal winner thank year wave sausage worth useful legal winner thank yellow'), + }; + + storageState.set('self_mnemonic', originalMnemonic); + storageState.set('self_private_key', originalSecret); + + let failTargetPrivateKeyWrite = true; + let failRollbackMnemonicWrite = true; + const storage = { + get: async (key: string) => storageState.get(key) ?? null, + set: async (key: string, value: string) => { + storageState.set(key, value); + if (key === 'self_private_key' && failTargetPrivateKeyWrite) { + failTargetPrivateKeyWrite = false; + throw new Error('write failed'); + } + if (key === 'self_mnemonic' && value === originalMnemonic && failRollbackMnemonicWrite) { + failRollbackMnemonicWrite = false; + throw new Error('mnemonic rollback failed'); + } + }, + remove: async (key: string) => { + storageState.delete(key); + }, + }; + + await expect(restoreStoredSecretSnapshot(storage, targetSnapshot)).rejects.toThrow('write failed'); + expect(storageState.get('self_mnemonic')).toBeUndefined(); + expect(storageState.get('self_private_key')).toBeUndefined(); + }); +});