fix(background): treat a wiped hardware wallet session as locked

extension/src/background/ducks/session.ts

@@ -201,7 +201,19 @@ export const buildHasPrivateKeySelector = (localStore: DataStorageAccess) =>
     const isHardwareWalletActive = await getIsHardwareWalletActive({
       localStore,
     });
-    if (isHardwareWalletActive && !session?.isHardwareWalletLocked) {
+    // A hardware-wallet session is unlocked only when it has not been
+    // idle-locked AND the session still holds a public key. The session
+    // store lives in `storage.session`, which the browser wipes on restart
+    // and on extension reload/update. After a wipe, `isHardwareWalletLocked`
+    // falls back to its `false` default, so without the public key check
+    // an empty session would read as unlocked. The popup would then show
+    // the unlock screen (no public key) while already treating the wallet
+    // as unlocked, and a correct password would never navigate away.
+    if (
+      isHardwareWalletActive &&
+      !session?.isHardwareWalletLocked &&
+      !!session?.publicKey
+    ) {
       return true;
     }
     if (!session?.hashKey?.key) {

extension/src/background/helpers/__tests__/session.test.ts

@@ -219,7 +219,44 @@ describe("buildHasPrivateKeySelector", () => {
     } as any);
 
     await expect(
-      selector({ session: { isHardwareWalletLocked: false } } as any),
+      selector({
+        session: { isHardwareWalletLocked: false, publicKey: "G-HW-KEY" },
+      } as any),
     ).resolves.toBe(true);
   });
+
+  it("returns false for idle-locked hardware-wallet sessions", async () => {
+    mockGetIsHardwareWalletActive.mockResolvedValue(true);
+    const selector = buildHasPrivateKeySelector({
+      getItem: jest.fn().mockResolvedValue(null),
+    } as any);
+
+    await expect(
+      selector({
+        session: { isHardwareWalletLocked: true, publicKey: "G-HW-KEY" },
+      } as any),
+    ).resolves.toBe(false);
+  });
+
+  // The browser wipes `storage.session` on restart and on extension
+  // reload/update. The rehydrated session has no public key and
+  // `isHardwareWalletLocked` falls back to `false`. That empty session
+  // must read as locked, or the unlock screen never navigates away.
+  it("returns false for a wiped hardware-wallet session with no public key", async () => {
+    mockGetIsHardwareWalletActive.mockResolvedValue(true);
+    const selector = buildHasPrivateKeySelector({
+      getItem: jest.fn().mockResolvedValue(null),
+    } as any);
+
+    await expect(
+      selector({
+        session: {
+          publicKey: "",
+          hashKey: { key: "" },
+          allAccounts: [],
+          isHardwareWalletLocked: false,
+        },
+      } as any),
+    ).resolves.toBe(false);
+  });
 });

extension/src/background/messageListener/__tests__/hardwareWalletWipedSessionUnlock.test.ts

@@ -0,0 +1,165 @@
+import { configureStore } from "@reduxjs/toolkit";
+import { combineReducers } from "redux";
+
+import { sessionSlice } from "background/ducks/session";
+import {
+  HAS_ACCOUNT_SUBSCRIPTION,
+  KEY_ID,
+  KEY_ID_LIST,
+} from "constants/localStorageTypes";
+
+import { confirmPassword } from "../handlers/confirmPassword";
+import { loadAccount } from "../handlers/loadAccount";
+
+// Persisting the store and broadcasting to other surfaces are side effects
+// that are not under test here.
+jest.mock("background/store", () => ({
+  flushSessionStore: jest.fn().mockResolvedValue(undefined),
+}));
+
+jest.mock("../helpers/broadcast-session-state", () => ({
+  broadcastSessionState: jest.fn().mockResolvedValue(undefined),
+}));
+
+jest.mock("@sentry/browser", () => ({
+  captureException: jest.fn(),
+}));
+
+const HW_PUBLIC_KEY = "GBHWLEDGERPUBLICKEY";
+const HW_KEY_ID = `hw:${HW_PUBLIC_KEY}`;
+const HOT_KEY_ID = "hot-key-id-0";
+const HOT_PUBLIC_KEY = "GBHOTWALLETPUBLICKEY";
+const PASSWORD = "correct-password";
+
+const buildLocalStore = (initial: Record<string, unknown>) => {
+  const data: Record<string, unknown> = { ...initial };
+  return {
+    getItem: jest.fn((key: string) => Promise.resolve(data[key] ?? null)),
+    setItem: jest.fn((key: string, value: unknown) => {
+      data[key] = value;
+      return Promise.resolve();
+    }),
+    remove: jest.fn((key: string) => {
+      delete data[key];
+      return Promise.resolve();
+    }),
+  };
+};
+
+// Only the hot (mnemonic) keystore can be unlocked with a password. A
+// hardware wallet key ID has no keystore, so loading it always fails.
+const buildKeyManager = () => ({
+  loadKey: jest.fn((keyId: string, password: string) => {
+    if (keyId === HOT_KEY_ID && password === PASSWORD) {
+      return Promise.resolve({
+        publicKey: HOT_PUBLIC_KEY,
+        privateKey: "SHOTWALLETSECRET",
+        extra: { mnemonicPhrase: "test mnemonic phrase", imported: false },
+      });
+    }
+    return Promise.reject(new Error("Could not load key"));
+  }),
+});
+
+// A fresh store with no preloaded state is what the background rebuilds
+// after the browser wipes `storage.session` (browser restart, extension
+// reload or update): no public key, no hash key and
+// `isHardwareWalletLocked: false`.
+const buildWipedSessionStore = () =>
+  configureStore({
+    reducer: combineReducers({ session: sessionSlice.reducer }),
+  });
+
+describe("unlocking a wiped session with a hardware wallet active", () => {
+  it("reports locked after the wipe, then unlocks the hardware wallet with the password", async () => {
+    const localStore = buildLocalStore({
+      [KEY_ID]: HW_KEY_ID,
+      [KEY_ID_LIST]: [HOT_KEY_ID, HW_KEY_ID],
+      // Already subscribed, so `logIn` does not call the indexer.
+      [HAS_ACCOUNT_SUBSCRIPTION]: { [HW_KEY_ID]: true },
+    });
+    const sessionStore = buildWipedSessionStore();
+    const keyManager = buildKeyManager();
+    const sessionTimer = {
+      startSession: jest.fn().mockResolvedValue(undefined),
+    };
+
+    // 1. The wiped session must read as locked. Before the fix this
+    //    returned `hasPrivateKey: true` with an empty public key, so the
+    //    unlock screen mounted already "unlocked" and never navigated.
+    const beforeUnlock = await loadAccount({
+      localStore: localStore as any,
+      sessionStore,
+    });
+    expect(beforeUnlock.publicKey).toBe("");
+    expect(beforeUnlock.hasPrivateKey).toBe(false);
+
+    // 2. The correct password unlocks the session.
+    const unlockResult = await confirmPassword({
+      request: { password: PASSWORD } as any,
+      localStore: localStore as any,
+      sessionStore,
+      keyManager: keyManager as any,
+      sessionTimer: sessionTimer as any,
+    });
+
+    expect(unlockResult).not.toHaveProperty("error");
+    expect(unlockResult).toMatchObject({
+      publicKey: HW_PUBLIC_KEY,
+      hasPrivateKey: true,
+    });
+
+    // The password is checked against the hot keystore, never against
+    // the hardware wallet key ID (which has no keystore).
+    expect(keyManager.loadKey.mock.calls[0]).toEqual([HOT_KEY_ID, PASSWORD]);
+    expect(keyManager.loadKey).not.toHaveBeenCalledWith(
+      HW_KEY_ID,
+      expect.anything(),
+    );
+    expect(sessionTimer.startSession).toHaveBeenCalledTimes(1);
+
+    // 3. The background now reports the hardware wallet as the unlocked
+    //    active account, so `hasPrivateKey` goes false -> true and the
+    //    unlock screen navigates.
+    const afterUnlock = await loadAccount({
+      localStore: localStore as any,
+      sessionStore,
+    });
+    expect(afterUnlock.publicKey).toBe(HW_PUBLIC_KEY);
+    expect(afterUnlock.hasPrivateKey).toBe(true);
+    expect(afterUnlock.allAccounts.map((a) => a.publicKey)).toEqual([
+      HOT_PUBLIC_KEY,
+      HW_PUBLIC_KEY,
+    ]);
+  });
+
+  it("stays locked when the password is wrong", async () => {
+    const localStore = buildLocalStore({
+      [KEY_ID]: HW_KEY_ID,
+      [KEY_ID_LIST]: [HOT_KEY_ID, HW_KEY_ID],
+      [HAS_ACCOUNT_SUBSCRIPTION]: { [HW_KEY_ID]: true },
+    });
+    const sessionStore = buildWipedSessionStore();
+    const sessionTimer = {
+      startSession: jest.fn().mockResolvedValue(undefined),
+    };
+
+    const unlockResult = await confirmPassword({
+      request: { password: "wrong-password" } as any,
+      localStore: localStore as any,
+      sessionStore,
+      keyManager: buildKeyManager() as any,
+      sessionTimer: sessionTimer as any,
+    });
+
+    expect(unlockResult).toEqual({ error: "Incorrect password" });
+    expect(sessionTimer.startSession).not.toHaveBeenCalled();
+
+    const afterAttempt = await loadAccount({
+      localStore: localStore as any,
+      sessionStore,
+    });
+    expect(afterAttempt.publicKey).toBe("");
+    expect(afterAttempt.hasPrivateKey).toBe(false);
+  });
+});

extension/src/background/messageListener/__tests__/userActivity.test.ts

@@ -1,3 +1,5 @@
+import { TEMPORARY_STORE_ID } from "constants/localStorageTypes";
+
 import { userActivity } from "../handlers/userActivity";
 
 const mockGetIsHardwareWalletActive = jest.fn();
@@ -18,7 +20,12 @@ const makeSessionStore = (state: any) =>
     getState: () => state,
   }) as any;
 
-const makeLocalStore = () => ({}) as any;
+const makeLocalStore = (temporaryStore: Record<string, string> = {}) =>
+  ({
+    getItem: jest.fn((key: string) =>
+      Promise.resolve(key === TEMPORARY_STORE_ID ? temporaryStore : null),
+    ),
+  }) as any;
 
 describe("userActivity handler", () => {
   beforeEach(() => {
@@ -37,17 +44,36 @@ describe("userActivity handler", () => {
     const result = await userActivity({
       sessionTimer,
       sessionStore,
-      localStore: makeLocalStore(),
+      localStore: makeLocalStore({ "key-id-0": "encrypted-blob" }),
     });
     expect(result).toEqual({ ok: true });
     expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1);
   });
 
+  it("does NOT reset the timer when the hash key exists but the temporary store is empty", async () => {
+    mockGetIsHardwareWalletActive.mockResolvedValue(false);
+    const sessionTimer = makeSessionTimer();
+    const sessionStore = makeSessionStore({
+      session: {
+        hashKey: { key: "abc" },
+        isHardwareWalletLocked: false,
+      },
+    });
+    const result = await userActivity({
+      sessionTimer,
+      sessionStore,
+      localStore: makeLocalStore({}),
+    });
+    expect(result).toEqual({ ok: false });
+    expect(sessionTimer.resetSession).not.toHaveBeenCalled();
+  });
+
   it("resets the timer when a hardware wallet is active and unlocked", async () => {
     mockGetIsHardwareWalletActive.mockResolvedValue(true);
     const sessionTimer = makeSessionTimer();
     const sessionStore = makeSessionStore({
       session: {
+        publicKey: "GBHW",
         hashKey: { key: "" },
         isHardwareWalletLocked: false,
       },
@@ -84,6 +110,7 @@ describe("userActivity handler", () => {
     const sessionTimer = makeSessionTimer();
     const sessionStore = makeSessionStore({
       session: {
+        publicKey: "GBHW",
         hashKey: { key: "" },
         isHardwareWalletLocked: true,
       },
@@ -96,4 +123,26 @@ describe("userActivity handler", () => {
     expect(result).toEqual({ ok: false });
     expect(sessionTimer.resetSession).not.toHaveBeenCalled();
   });
+
+  // After the browser wipes `storage.session`, the session has no public
+  // key and `isHardwareWalletLocked` falls back to `false`. That session
+  // is locked, so activity must not rearm the idle alarm.
+  it("does NOT reset the timer for a wiped hardware-wallet session with no public key", async () => {
+    mockGetIsHardwareWalletActive.mockResolvedValue(true);
+    const sessionTimer = makeSessionTimer();
+    const sessionStore = makeSessionStore({
+      session: {
+        publicKey: "",
+        hashKey: { key: "" },
+        isHardwareWalletLocked: false,
+      },
+    });
+    const result = await userActivity({
+      sessionTimer,
+      sessionStore,
+      localStore: makeLocalStore(),
+    });
+    expect(result).toEqual({ ok: false });
+    expect(sessionTimer.resetSession).not.toHaveBeenCalled();
+  });
 });

extension/src/background/messageListener/handlers/userActivity.ts

@@ -1,11 +1,9 @@
 import { Store } from "redux";
 
-import { getIsHardwareWalletActive } from "background/helpers/account";
 import { DataStorageAccess } from "background/helpers/dataStorageAccess";
 import { SessionTimer } from "background/helpers/session";
 import {
-  hashKeySelector,
-  isHardwareWalletLockedSelector,
+  buildHasPrivateKeySelector,
   SessionState,
 } from "background/ducks/session";
 
@@ -28,8 +26,9 @@ import {
  * the idle alarm. A ping that arrives on a locked wallet is dropped:
  * there is no live session to extend.
  *
- * Unlocked = either a hot-wallet session (`hashKey` is set) or an
- * active hardware-wallet session that has not been idle-locked.
+ * "Unlocked" uses the same `buildHasPrivateKeySelector` rule as
+ * `loadAccount` and `saveSettings`, so the idle alarm and the popup
+ * never disagree about whether the wallet is unlocked.
  */
 export const userActivity = async ({
   sessionTimer,
@@ -40,13 +39,12 @@ export const userActivity = async ({
   sessionStore: Store;
   localStore: DataStorageAccess;
 }) => {
-  const state = sessionStore.getState() as SessionState;
-  const hashKey = hashKeySelector(state);
-  const hotUnlocked = !!hashKey?.key;
-  const isHwActive = await getIsHardwareWalletActive({ localStore });
-  const hwUnlocked = isHwActive && !isHardwareWalletLockedSelector(state);
+  const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore);
+  const isUnlocked = await hasPrivateKeySelector(
+    sessionStore.getState() as SessionState,
+  );
 
-  if (!hotUnlocked && !hwUnlocked) {
+  if (!isUnlocked) {
     return { ok: false };
   }