fix: allow isAuthenticating updates on frozen SecureKeychain

[grvgoel81]

Description

Biometric / fingerprint unlock fails on the Login screen with:

TypeError: Cannot assign to read-only property 'isAuthenticating'

Root cause: SecureKeychain.init() calls Object.freeze(instance) on the singleton. During unlock, getGenericPassword() / getSecureItem() set instance.isAuthenticating = true|false so LockManagerService does not auto-lock mid-auth. That write mutates a frozen object. After the React Native 0.81 → 0.83 / Hermes upgrade (shipped with the 8.4 line), this assignment throws instead of failing silently, so Login surfaces the error and unlock cannot complete.

Solution: Keep Object.freeze(instance) (salt / instance integrity), but move isAuthenticating into the existing WeakMap-backed private state and expose it via getter/setter. Callers keep the same API; the flag can change without mutating frozen own-data properties.

Changelog

CHANGELOG entry: ixed fingerprint and biometric unlock failing with a read-only isAuthenticating error after the React Native 0.83 upgrade

Related issues

Fixes:

Manual testing steps

Feature: biometric unlock after SecureKeychain freeze fix

  Scenario: user unlocks wallet with fingerprint after app lock
    Given the app is installed with biometrics / fingerprint enabled for MetaMask
    And the wallet is locked and the Login screen is visible

    When the user authenticates with fingerprint / biometrics
    Then the wallet unlocks successfully
    And the Login screen does not show "Cannot assign to read-only property 'isAuthenticating'"

  Scenario: user can still unlock with password if biometrics is cancelled
    Given the app is locked and biometrics are enabled
    When the user cancels the biometric prompt
    And the user enters their password and taps Unlock
    Then the wallet unlocks successfully

Screenshots/Recordings

Before

screen-20260716-102617-1784190357463.mp4

After

fingerprint.mp4

Pre-merge author checklist

• I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I've included tests if applicable
• I've documented my code using JSDoc format if applicable
• I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

• I've tested on Android
  • Ideally on a mid-range device; emulator is acceptable
• I've tested with a power user scenario
  • Use these power-user SRPs to import wallets with many accounts and tokens
• I've instrumented key operations with Sentry traces for production performance metrics
  • See trace() for usage and addToken for an example

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

---

Note

Medium Risk
Touches login/unlock and auto-lock coordination during keychain auth; behavior is intended to be unchanged but failures would block wallet access.

Overview
Fixes biometric/fingerprint unlock failing on Login with Cannot assign to read-only property 'isAuthenticating' after the RN 0.83 / Hermes upgrade, where writes to frozen objects now throw.

SecureKeychain.init() still Object.freezes the singleton, but isAuthenticating is no longer an own property on SecureKeychainEncryptor. It lives in the existing WeakMap private state (alongside code) and is exposed through a getter/setter, so getGenericPassword / getSecureItem can flip the flag during auth without mutating the frozen instance. LockManagerService and other callers keep the same instance.isAuthenticating API.

Adds a unit test that getSecureItem resets isAuthenticating to false on the frozen singleton after a mocked keychain read.

^{Reviewed by Cursor Bugbot for commit 9c67f54. Configure here.}

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[metamask-ci]

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

• Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
• Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

[github-actions]

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File	7d	15d	30d
app/core/SecureKeychain.test.ts	0/122	0/162	0/371

AI-detected flaky patterns

app/core/SecureKeychain.test.ts

• J10 — jest.spyOn without restoreAllMocks in afterEach (medium)
  • Multiple jest.spyOn calls (on SecureKeychain.resetGenericPassword in first describe and on QuickCrypto.subtle.decrypt in the second describe) lack any afterEach with jest.restoreAllMocks(). Spies persist across tests and can alter behavior in subsequent tests, especially since beforeEach only uses clearAllMocks() which does not restore original implementations. This matches J10 exactly. Historical data showed 0 failures so hint not used for detection.
  • Suggested fix in app/core/SecureKeychain.test.ts:

    -  beforeEach(() => {
    -    jest.clearAllMocks();
    -    SecureKeychain.init('test_salt');
    -  });
    -
    +  beforeEach(() => {
    +    jest.clearAllMocks();
    +    SecureKeychain.init('test_salt');
    +  });
    +
    +  afterEach(() => {
    +    jest.restoreAllMocks();
    +  });
    +

This check is informational only and does not block merging.

[juanmigdr]

LGTM, record time 🚀

[github-actions]

🔍 Smart E2E Test Selection

• Selected E2E tags: SmokeAccounts
• Selected Performance tags: @PerformanceLogin
• Risk Level: low
• AI Confidence: 85%

click to see 🤖 AI reasoning details

E2E Test Selection:
The changes to SecureKeychain.ts are a pure TypeScript refactoring:

1. Added a TypeScript interface SecureKeychainPrivateState for type safety
2. Moved isAuthenticating from a public class field to a getter/setter backed by the existing WeakMap private state
3. Added a #getPrivateState() private method with null-safety guard
4. No behavioral changes — the property still works identically from the outside

The test file adds one new test case verifying the refactored isAuthenticating toggle behavior on the frozen singleton.

SecureKeychain is used in authentication/login flows, onboarding, and account security. Since this is a refactoring of the authentication keychain module, SmokeAccounts is the most relevant tag to verify that account security flows (SRP reveal, account management) still work correctly after the refactoring. The login/unlock path is also exercised as part of account tests.

No other tags are needed because:

• No UI changes were made
• No logic changes were made — only structural TypeScript improvements
• The change is isolated to the SecureKeychainEncryptor class internals
• No new features or flows were introduced

Performance Test Selection:
SecureKeychain is directly involved in the login/unlock flow (password encryption/decryption for biometric and passcode authentication). While the change is a pure refactoring, the @PerformanceLogin tag covers the unlock flow that relies on SecureKeychain. The refactoring moves isAuthenticating to a WeakMap-backed getter/setter which could theoretically have minor performance implications, though in practice the impact should be negligible. Including this tag as a conservative measure to verify no regression in login performance.

View GitHub Actions results

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
83.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

❌ 1 test failed · 5 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (1)

@metamask-mobile-platform

Test	Platform	Device	Reason	Recording
Connect to Uniswap dapp, edit accounts, choose another account, and skip Solana popup	Android	Google Pixel 8 Pro (v14.0)	Test error	📹 Watch

✅ Passed Tests (4)

Test	Platform	Device	Duration	Team	Recording
Cold Start: Measure ColdStart To Login Screen	Android	Google Pixel 8 Pro (v14.0)	3.90s	@metamask-mobile-platform	📹 Watch
Measure Warm Start: Login To Wallet Screen	Android	Google Pixel 8 Pro (v14.0)	1.36s	@metamask-mobile-platform	📹 Watch
Asset View, SRP 1 + SRP 2 + SRP 3	Android	Google Pixel 8 Pro (v14.0)	2.73s	@assets-dev-team	📹 Watch
Measure Warm Start: Warm Start to Login Screen	Android	Google Pixel 8 Pro (v14.0)	1.84s	@metamask-mobile-platform	📹 Watch

---

Branch: fix/secure-keychain-isauthenticating-freeze · Build: Normal · Commit: cac5ba8 · View full run