From edd69d3ddc836967dc14a6697fe43dd87868f32c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 18:28:26 +0000 Subject: [PATCH 1/3] fix(background): treat a wiped hardware wallet session as locked When a hardware wallet account is active, buildHasPrivateKeySelector returned true whenever isHardwareWalletLocked was false. The browser wipes storage.session on restart and on extension reload/update. The rehydrated session then has no public key, and isHardwareWalletLocked falls back to false, so the empty session read as unlocked. The popup saved hasPrivateKey: true, saw no public key, and routed to the unlock screen. The unlock screen only navigates on a false -> true change of hasPrivateKey, so a correct password did nothing and showed no error. Require a session public key before a hardware wallet session counts as unlocked. Add selector tests for the wiped and idle-locked cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA --- extension/src/background/ducks/session.ts | 14 ++++++- .../helpers/__tests__/session.test.ts | 39 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/extension/src/background/ducks/session.ts b/extension/src/background/ducks/session.ts index aea5410407..d946fa1cf3 100644 --- a/extension/src/background/ducks/session.ts +++ b/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) { diff --git a/extension/src/background/helpers/__tests__/session.test.ts b/extension/src/background/helpers/__tests__/session.test.ts index a8e77ce57c..a5414b6fed 100644 --- a/extension/src/background/helpers/__tests__/session.test.ts +++ b/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); + }); }); From 150dc64d8c84dc302f3f8803be600c8a21c531ae Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 21:14:25 +0000 Subject: [PATCH 2/3] test(background): cover unlock of a wiped hardware wallet session Run the real loadAccount and confirmPassword handlers against a fresh session store with a hardware wallet active. Check that loadAccount reports the wiped session as locked, that the correct password unlocks it with the hardware wallet public key, and that a wrong password leaves it locked. Both cases fail without the selector fix. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA --- .../hardwareWalletWipedSessionUnlock.test.ts | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 extension/src/background/messageListener/__tests__/hardwareWalletWipedSessionUnlock.test.ts diff --git a/extension/src/background/messageListener/__tests__/hardwareWalletWipedSessionUnlock.test.ts b/extension/src/background/messageListener/__tests__/hardwareWalletWipedSessionUnlock.test.ts new file mode 100644 index 0000000000..21cf467f28 --- /dev/null +++ b/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) => { + const data: Record = { ...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); + }); +}); From 51d4316a14aaa2822af4a34ebd67a95773d4fe9d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 21:34:27 +0000 Subject: [PATCH 3/3] refactor(background): use buildHasPrivateKeySelector in userActivity userActivity had its own copy of the unlock check. Use the shared selector, as saveSettings does, so the idle alarm and loadAccount cannot drift apart. Behavior changes: - A wiped hardware wallet session (no public key) no longer rearms the idle alarm. - A hot wallet session with a hash key but an empty temporary store no longer rearms the idle alarm. loadAccount already reports it locked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA --- .../__tests__/userActivity.test.ts | 53 ++++++++++++++++++- .../messageListener/handlers/userActivity.ts | 20 ++++--- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/extension/src/background/messageListener/__tests__/userActivity.test.ts b/extension/src/background/messageListener/__tests__/userActivity.test.ts index 4d17bd3cce..c1d1f71769 100644 --- a/extension/src/background/messageListener/__tests__/userActivity.test.ts +++ b/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 = {}) => + ({ + 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(); + }); }); diff --git a/extension/src/background/messageListener/handlers/userActivity.ts b/extension/src/background/messageListener/handlers/userActivity.ts index 6c50e1acbc..b4330b9458 100644 --- a/extension/src/background/messageListener/handlers/userActivity.ts +++ b/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 }; }