From 357840873683d1df2af14e587d57a6406722b0d0 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 21 May 2026 14:21:15 -0700 Subject: [PATCH 01/21] Replace 24h absolute auto-lock with configurable idle timer Replaces the hardcoded 24-hour 'session length' alarm with a pure idle-based auto-lock timer. The browser session locks after a configurable number of minutes of user inactivity (1, 5, 15, 30, 60), defaulting to 15. Genuine user interaction in any extension page pings the background via a new USER_ACTIVITY message, which rearms the alarm. - @shared/constants/autoLock.ts: single source of truth for valid timeouts, default, and coercion helpers. - SessionTimer: rewritten as a class that reads the persisted timeout on every reset; same-name browser.alarms.create atomically replaces the in-flight deadline. - saveSettings: validates and persists the new field; when the new timeout is shorter than the elapsed idle time on an unlocked wallet, locks immediately rather than scheduling an alarm in the past. - userActivity handler: gated by isFromExtensionPage in popupMessageListener so dApp content scripts cannot extend a session. Rejects pings when the wallet is locked. - useActivityPing + ActivityTracker: mousedown/keydown/touchstart/wheel listeners on window with 5s leading-edge throttle; only active when the wallet is unlocked. - Preferences UI: dropdown driven by VALID_AUTO_LOCK_TIMEOUT_MINUTES. - Tests cover SessionTimer, userActivity, save/load validation, and the elapsed-idle short-circuit; mock fixtures updated to include the new field. Closes #2082 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/api/internal.ts | 21 +- @shared/api/types/message-request.ts | 9 +- @shared/api/types/types.ts | 2 + @shared/constants/autoLock.ts | 27 ++ @shared/constants/services.ts | 1 + extension/src/background/ducks/session.ts | 32 ++- .../helpers/__tests__/session.test.ts | 84 +++++++ extension/src/background/helpers/session.ts | 66 ++++- extension/src/background/index.ts | 4 +- .../__tests__/createAccount.test.ts | 10 +- .../__tests__/handleSignedHwPayload.test.ts | 70 ++++++ .../__tests__/loadSaveSettings.test.ts | 234 +++++++++++++++++- .../__tests__/userActivity.test.ts | 81 ++++++ .../handlers/confirmPassword.ts | 6 + .../messageListener/handlers/createAccount.ts | 2 +- .../handlers/handleSignedHwPayload.ts | 13 +- .../messageListener/handlers/loadSettings.ts | 6 + .../handlers/migrateAccounts.ts | 2 +- .../handlers/recoverAccount.ts | 2 +- .../messageListener/handlers/saveSettings.ts | 74 ++++++ .../messageListener/handlers/signOut.ts | 6 + .../messageListener/handlers/userActivity.ts | 46 ++++ .../helpers/login-all-accounts.ts | 2 +- .../messageListener/helpers/test-helpers.ts | 35 ++- .../messageListener/popupMessageListener.ts | 10 + extension/src/constants/localStorageTypes.ts | 1 + extension/src/popup/App.tsx | 2 + .../components/ActivityTracker/index.tsx | 17 ++ .../popup/ducks/__tests__/settings.test.ts | 92 +++++++ extension/src/popup/ducks/accountServices.ts | 4 + extension/src/popup/ducks/settings.ts | 40 ++- .../hooks/__tests__/useActivityPing.test.ts | 97 ++++++++ .../popup/helpers/hooks/useActivityPing.ts | 51 ++++ extension/src/popup/views/IntegrationTest.tsx | 3 + .../src/popup/views/Preferences/index.tsx | 56 ++++- .../popup/views/__tests__/Account.test.tsx | 8 + .../views/__tests__/AccountCreator.test.tsx | 2 + .../views/__tests__/AccountHistory.test.tsx | 2 + .../popup/views/__tests__/AddFunds.test.tsx | 2 + .../views/__tests__/GrantAccess.test.tsx | 2 + .../views/__tests__/ManageAssets.test.tsx | 2 + .../views/__tests__/SignTransaction.test.tsx | 13 + .../src/popup/views/__tests__/Swap.test.tsx | 2 + 43 files changed, 1203 insertions(+), 38 deletions(-) create mode 100644 @shared/constants/autoLock.ts create mode 100644 extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts create mode 100644 extension/src/background/messageListener/__tests__/userActivity.test.ts create mode 100644 extension/src/background/messageListener/handlers/userActivity.ts create mode 100644 extension/src/popup/components/ActivityTracker/index.tsx create mode 100644 extension/src/popup/ducks/__tests__/settings.test.ts create mode 100644 extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts create mode 100644 extension/src/popup/helpers/hooks/useActivityPing.ts diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index f8eb838d50..8597d7101b 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -13,6 +13,10 @@ import { } from "stellar-sdk"; import BigNumber from "bignumber.js"; import { INDEXER_URL, INDEXER_V2_URL } from "@shared/constants/mercury"; +import { + AutoLockTimeoutMinutes, + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, +} from "@shared/constants/autoLock"; import { AssetListResponse, AssetsListItem, @@ -1618,13 +1622,15 @@ export const saveSettings = async ({ isMemoValidationEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, }: { activePublicKey: string; isDataSharingAllowed: boolean; isMemoValidationEnabled: boolean; isHideDustEnabled: boolean; isOpenSidebarByDefault: boolean; -}): Promise => { + autoLockTimeoutMinutes: AutoLockTimeoutMinutes; +}): Promise => { let response = { allowList: DEFAULT_ALLOW_LIST, isDataSharingAllowed: false, @@ -1638,19 +1644,22 @@ export const saveSettings = async ({ isNonSSLEnabled: false, isHideDustEnabled: true, isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, error: "", hiddenAssets: {}, + wasLocked: false, }; try { - response = await sendMessageToBackground({ + response = (await sendMessageToBackground({ activePublicKey, isDataSharingAllowed, isMemoValidationEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, type: SERVICE_TYPES.SAVE_SETTINGS, - }); + })) as unknown as typeof response; } catch (e) { console.error(e); } @@ -1866,7 +1875,11 @@ export const loadSettings = (): Promise< sendMessageToBackground({ activePublicKey: null, type: SERVICE_TYPES.LOAD_SETTINGS, - }); + }) as unknown as Promise< + Settings & + IndexerSettings & + ExperimentalFeatures & { assetsLists: AssetsLists } + >; export const loadBackendSettings = async (): Promise<{ isSorobanPublicEnabled: boolean; diff --git a/@shared/api/types/message-request.ts b/@shared/api/types/message-request.ts index 196085933a..8856367d1a 100644 --- a/@shared/api/types/message-request.ts +++ b/@shared/api/types/message-request.ts @@ -11,6 +11,7 @@ import { CollectibleKey, } from "./types"; import { AssetsListItem } from "@shared/constants/soroban/asset-list"; +import { AutoLockTimeoutMinutes } from "@shared/constants/autoLock"; export interface TokenToAdd { domain: string; @@ -282,6 +283,11 @@ export interface SaveSettingsMessage extends BaseMessage { isMemoValidationEnabled: boolean; isDataSharingAllowed: boolean; isOpenSidebarByDefault: boolean; + autoLockTimeoutMinutes: AutoLockTimeoutMinutes; +} + +export interface UserActivityMessage extends BaseMessage { + type: SERVICE_TYPES.USER_ACTIVITY; } export interface SaveExperimentalFeaturesMessage extends BaseMessage { @@ -547,4 +553,5 @@ export type ServiceMessageRequest = | GetHiddenCollectiblesMessage | MarkQueueActiveMessage | OpenSidebarMessage - | RejectSigningRequestMessage; + | RejectSigningRequestMessage + | UserActivityMessage; diff --git a/@shared/api/types/types.ts b/@shared/api/types/types.ts index c2718762fe..684e1f480e 100644 --- a/@shared/api/types/types.ts +++ b/@shared/api/types/types.ts @@ -13,6 +13,7 @@ import { AssetsLists, AssetsListItem, } from "../../constants/soroban/asset-list"; +import { AutoLockTimeoutMinutes } from "../../constants/autoLock"; export enum ActionStatus { IDLE = "IDLE", @@ -189,6 +190,7 @@ export interface Preferences { networksList: NetworkDetails[]; isHideDustEnabled: boolean; isOpenSidebarByDefault: boolean; + autoLockTimeoutMinutes: AutoLockTimeoutMinutes; error: string; } diff --git a/@shared/constants/autoLock.ts b/@shared/constants/autoLock.ts new file mode 100644 index 0000000000..bbb9426e72 --- /dev/null +++ b/@shared/constants/autoLock.ts @@ -0,0 +1,27 @@ +/** + * Single source of truth for the idle auto-lock timeout feature. + * + * The browser session is locked after this many minutes of user + * inactivity across all extension surfaces (popup, sidebar, standalone + * signing windows, grant-access windows). Any user interaction inside + * an extension page resets the timer. + */ +export const VALID_AUTO_LOCK_TIMEOUT_MINUTES = [1, 5, 15, 30, 60] as const; + +export type AutoLockTimeoutMinutes = + (typeof VALID_AUTO_LOCK_TIMEOUT_MINUTES)[number]; + +export const DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES: AutoLockTimeoutMinutes = 15; + +export const isValidAutoLockTimeoutMinutes = ( + value: unknown, +): value is AutoLockTimeoutMinutes => + typeof value === "number" && + (VALID_AUTO_LOCK_TIMEOUT_MINUTES as readonly number[]).includes(value); + +export const coerceAutoLockTimeoutMinutes = ( + value: unknown, +): AutoLockTimeoutMinutes => + isValidAutoLockTimeoutMinutes(value) + ? value + : DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES; diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index 098e98d20b..f7d9297bb4 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -70,6 +70,7 @@ export enum SERVICE_TYPES { CLEAR_RECENT_PROTOCOLS = "CLEAR_RECENT_PROTOCOLS", GET_DISCOVER_WELCOME_SEEN = "GET_DISCOVER_WELCOME_SEEN", DISMISS_DISCOVER_WELCOME = "DISMISS_DISCOVER_WELCOME", + USER_ACTIVITY = "USER_ACTIVITY", } // SIDEBAR_NAVIGATE is a plain string constant (not in an enum) because it is diff --git a/extension/src/background/ducks/session.ts b/extension/src/background/ducks/session.ts index 512293776a..13b5a2a00e 100644 --- a/extension/src/background/ducks/session.ts +++ b/extension/src/background/ducks/session.ts @@ -58,6 +58,7 @@ const initialState: InitialState = { }, allAccounts: [] as Account[], migratedMnemonicPhrase: "", + isHardwareWalletLocked: false, }; interface UiData { @@ -70,6 +71,12 @@ interface AppData { privateKey?: string; hashKey?: { key: string }; password?: string; + // True once the idle auto-lock alarm fires on a hardware-wallet-active + // session. Hot-wallet (mnemonic) sessions are gated by `hashKey` + // instead, which `timeoutAccountAccess` already clears. HW sessions + // need a dedicated flag because `getIsHardwareWalletActive` is stored + // in `localStore` and is not cleared by the lock path. + isHardwareWalletLocked?: boolean; } export const sessionSlice = createSlice({ @@ -108,6 +115,19 @@ export const sessionSlice = createSlice({ }, password: "", }), + // Idle auto-lock for hardware-wallet sessions. `timeoutAccountAccess` + // clears the hot-wallet `hashKey`, but hardware-wallet "unlocked" + // state is read off `localStore.isHardwareWalletActive` and is + // unaffected by that — so without this flag, the idle alarm firing + // on an HW-only session would be a silent no-op. + lockHardwareWallet: (state) => ({ + ...state, + isHardwareWalletLocked: true, + }), + unlockHardwareWallet: (state) => ({ + ...state, + isHardwareWalletLocked: false, + }), updateAccountName: ( state, action: { payload: { publicKey: string; updatedAccountName: string } }, @@ -155,6 +175,8 @@ export const { logOut, setActiveHashKey, timeoutAccountAccess, + lockHardwareWallet, + unlockHardwareWallet, setMigratedMnemonicPhrase, updateAccountName, }, @@ -178,9 +200,17 @@ export const buildHasPrivateKeySelector = (localStore: DataStorageAccess) => const isHardwareWalletActive = await getIsHardwareWalletActive({ localStore, }); - return isHardwareWalletActive || !!session?.hashKey?.key; + if (isHardwareWalletActive && !session?.isHardwareWalletLocked) { + return true; + } + return !!session?.hashKey?.key; }); +export const isHardwareWalletLockedSelector = createSelector( + sessionSelector, + (session) => !!session?.isHardwareWalletLocked, +); + export const hashKeySelector = createSelector( sessionSelector, (session) => session.hashKey, diff --git a/extension/src/background/helpers/__tests__/session.test.ts b/extension/src/background/helpers/__tests__/session.test.ts index eb2f619194..5b281aa1bd 100644 --- a/extension/src/background/helpers/__tests__/session.test.ts +++ b/extension/src/background/helpers/__tests__/session.test.ts @@ -2,7 +2,12 @@ import { deriveKeyFromString, encryptHashString, decryptHashString, + SessionTimer, + SESSION_ALARM_NAME, } from "../session"; +import browser from "webextension-polyfill"; +import { AUTO_LOCK_TIMEOUT_MINUTES_ID } from "constants/localStorageTypes"; +import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; describe("session", () => { it("should be able to encrypt and decrypt a string", async () => { @@ -85,3 +90,82 @@ describe("session", () => { expect(areEqual).toBe(false); }); }); + +describe("SessionTimer", () => { + const createMock = jest.fn().mockResolvedValue(undefined); + const clearMock = jest.fn().mockResolvedValue(undefined); + + beforeEach(() => { + createMock.mockClear(); + clearMock.mockClear(); + (browser as any).alarms = { create: createMock, clear: clearMock }; + }); + + const makeLocalStore = (stored: unknown) => + ({ + getItem: jest.fn().mockImplementation((key: string) => { + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(stored); + return Promise.resolve(null); + }), + setItem: jest.fn(), + remove: jest.fn(), + }) as any; + + it("resetSession arms the alarm using the stored timeout", async () => { + const timer = new SessionTimer(makeLocalStore(30)); + await timer.resetSession(); + expect(createMock).toHaveBeenCalledWith(SESSION_ALARM_NAME, { + delayInMinutes: 30, + }); + }); + + it("startSession is an alias for resetSession", async () => { + const timer = new SessionTimer(makeLocalStore(5)); + await timer.startSession(); + expect(createMock).toHaveBeenCalledWith(SESSION_ALARM_NAME, { + delayInMinutes: 5, + }); + }); + + it("falls back to the default when no timeout is persisted", async () => { + const timer = new SessionTimer(makeLocalStore(null)); + await timer.resetSession(); + expect(createMock).toHaveBeenCalledWith(SESSION_ALARM_NAME, { + delayInMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + }); + }); + + it("falls back to the default when the persisted value is invalid", async () => { + const timer = new SessionTimer(makeLocalStore(7)); + await timer.resetSession(); + expect(createMock).toHaveBeenCalledWith(SESSION_ALARM_NAME, { + delayInMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + }); + }); + + it("re-reads the persisted timeout on every reset", async () => { + const localStore = { + getItem: jest + .fn() + .mockResolvedValueOnce(15) + .mockResolvedValueOnce(60), + setItem: jest.fn(), + remove: jest.fn(), + } as any; + const timer = new SessionTimer(localStore); + await timer.resetSession(); + await timer.resetSession(); + expect(createMock).toHaveBeenNthCalledWith(1, SESSION_ALARM_NAME, { + delayInMinutes: 15, + }); + expect(createMock).toHaveBeenNthCalledWith(2, SESSION_ALARM_NAME, { + delayInMinutes: 60, + }); + }); + + it("stopSession clears the alarm", async () => { + const timer = new SessionTimer(makeLocalStore(15)); + await timer.stopSession(); + expect(clearMock).toHaveBeenCalledWith(SESSION_ALARM_NAME); + }); +}); diff --git a/extension/src/background/helpers/session.ts b/extension/src/background/helpers/session.ts index 8709b595f3..7b18198745 100644 --- a/extension/src/background/helpers/session.ts +++ b/extension/src/background/helpers/session.ts @@ -6,26 +6,65 @@ import { hashKeySelector, SessionState, timeoutAccountAccess, + lockHardwareWallet, } from "../ducks/session"; import { DataStorageAccess } from "./dataStorageAccess"; -import { TEMPORARY_STORE_ID } from "../../constants/localStorageTypes"; +import { + AUTO_LOCK_TIMEOUT_MINUTES_ID, + TEMPORARY_STORE_ID, +} from "../../constants/localStorageTypes"; +import { + AutoLockTimeoutMinutes, + coerceAutoLockTimeoutMinutes, +} from "@shared/constants/autoLock"; import { encode, decode } from "./base64-arraybuffer"; -// 24 hours -const SESSION_LENGTH = 60 * 24; export const SESSION_ALARM_NAME = "session-timer"; +/** + * Idle-based auto-lock timer. + * + * The browser session is locked after a configurable number of minutes + * of user inactivity. Any genuine user interaction in an extension page + * pings the background, which calls `resetSession()` to rearm the alarm + * at `now + configured timeout`. The alarm is implemented as a named + * `browser.alarms` entry, so creating it with the same name replaces + * any in-flight deadline atomically — no separate clear step needed. + */ export class SessionTimer { - duration = 1000 * 60 * SESSION_LENGTH; - runningTimeout: null | ReturnType = null; - constructor(duration?: number) { - this.duration = duration || this.duration; + private readonly localStore: DataStorageAccess; + + constructor(localStore: DataStorageAccess) { + this.localStore = localStore; } - startSession() { - browser?.alarms.create(SESSION_ALARM_NAME, { - delayInMinutes: SESSION_LENGTH, - }); + private async getTimeoutMinutes(): Promise { + const stored = await this.localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID); + return coerceAutoLockTimeoutMinutes(stored); + } + + /** + * (Re)arm the auto-lock alarm. Reads the persisted timeout on every + * call so that settings changes take effect immediately. + */ + async resetSession() { + const delayInMinutes = await this.getTimeoutMinutes(); + await browser?.alarms.create(SESSION_ALARM_NAME, { delayInMinutes }); + } + + /** + * Alias kept so unlock paths can read naturally + * (`sessionTimer.startSession()`). + */ + async startSession() { + await this.resetSession(); + } + + /** + * Cancel any pending auto-lock alarm. Used by explicit sign-out. + */ + async stopSession() { + await browser?.alarms.clear(SESSION_ALARM_NAME); } } @@ -266,5 +305,10 @@ export const clearSession = async ({ sessionStore, }: ClearSession) => { sessionStore.dispatch(timeoutAccountAccess()); + // Locks hardware-wallet sessions too. `timeoutAccountAccess` clears + // the hot-wallet hashKey, but HW unlocked-state is read from + // `localStore.isHardwareWalletActive` and is unaffected — so without + // this dispatch the idle alarm would be a no-op on HW-only sessions. + sessionStore.dispatch(lockHardwareWallet()); await localStore.remove(TEMPORARY_STORE_ID); }; diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 8f56703919..991838b940 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -53,7 +53,9 @@ import { } from "@stellar/typescript-wallet-sdk-km"; import { BrowserStorageConfigParams } from "@stellar/typescript-wallet-sdk-km/lib/Plugins/BrowserStorageFacade"; -const sessionTimer = new SessionTimer(); +const sessionTimer = new SessionTimer( + dataStorageAccess(browserLocalStorage), +); export const initContentScriptMessageListener = () => { browser?.runtime?.onMessage?.addListener((message) => { diff --git a/extension/src/background/messageListener/__tests__/createAccount.test.ts b/extension/src/background/messageListener/__tests__/createAccount.test.ts index 44a6dd24bd..37adb37024 100644 --- a/extension/src/background/messageListener/__tests__/createAccount.test.ts +++ b/extension/src/background/messageListener/__tests__/createAccount.test.ts @@ -47,7 +47,7 @@ describe("Create account message listener", () => { mockSessionStore, mockDataStorage, mockKeyManager, - testAlarm, + testAlarm as any, { id: "fake-extension-id" }, )) as Awaited>; @@ -74,7 +74,7 @@ describe("Create account message listener", () => { mockSessionStore, mockDataStorage, mockKeyManager, - testAlarm, + testAlarm as any, { id: "fake-extension-id" }, )) as Awaited>; const keyId = await mockDataStorage.getItem(KEY_ID); @@ -88,7 +88,7 @@ describe("Create account message listener", () => { mockSessionStore, mockDataStorage, mockKeyManager, - testAlarm, + testAlarm as any, { id: "fake-extension-id" }, )) as Awaited>; const secondKeyId = await mockDataStorage.getItem(KEY_ID); @@ -111,7 +111,7 @@ describe("Create account message listener", () => { mockSessionStore, mockDataStorage, mockKeyManager, - testAlarm, + testAlarm as any, { id: "fake-extension-id" }, )) as Awaited>; const keyId = await mockDataStorage.getItem(KEY_ID); @@ -125,7 +125,7 @@ describe("Create account message listener", () => { mockSessionStore, mockDataStorage, mockKeyManager, - testAlarm, + testAlarm as any, { id: "fake-extension-id" }, )) as Awaited>; const secondKeyId = await mockDataStorage.getItem(KEY_ID); diff --git a/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts b/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts new file mode 100644 index 0000000000..8f3d4561e4 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts @@ -0,0 +1,70 @@ +import { handleSignedHwPayload } from "../handlers/handleSignedHwPayload"; +import { + isHardwareWalletLockedSelector, + lockHardwareWallet, + sessionSlice, + unlockHardwareWallet, +} from "background/ducks/session"; + +jest.mock("@sentry/browser", () => ({ + captureException: jest.fn(), +})); + +const makeSessionTimer = (onReset?: () => void) => + ({ + resetSession: jest.fn().mockImplementation(async () => { + onReset?.(); + }), + startSession: jest.fn().mockResolvedValue(undefined), + stopSession: jest.fn().mockResolvedValue(undefined), + }) as any; + +describe("handleSignedHwPayload", () => { + it("resets the session before resolving the queued hardware-wallet payload", async () => { + const callOrder: string[] = []; + const response = jest.fn(() => callOrder.push("response")); + const sessionTimer = makeSessionTimer(() => callOrder.push("reset")); + const responseQueue = [{ uuid: "uuid-1", response }]; + + const result = await handleSignedHwPayload({ + request: { + uuid: "uuid-1", + signedPayload: "signed-xdr", + } as any, + responseQueue: responseQueue as any, + sessionTimer, + }); + + expect(result).toEqual({}); + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + expect(response).toHaveBeenCalledWith("signed-xdr"); + expect(callOrder).toEqual(["reset", "response"]); + expect(responseQueue).toEqual([]); + }); + + it("still records user activity before returning an error for a missing uuid", async () => { + const sessionTimer = makeSessionTimer(); + + const result = await handleSignedHwPayload({ + request: { signedPayload: "signed-xdr" } as any, + responseQueue: [] as any, + sessionTimer, + }); + + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + expect(result).toEqual({ error: "Transaction not found" }); + }); + + it("tracks hardware-wallet lock and unlock state transitions", () => { + let state = { session: sessionSlice.reducer(undefined, { type: "init" }) }; + expect(isHardwareWalletLockedSelector(state)).toBe(false); + + state = { session: sessionSlice.reducer(state.session, lockHardwareWallet()) }; + expect(isHardwareWalletLockedSelector(state)).toBe(true); + + state = { + session: sessionSlice.reducer(state.session, unlockHardwareWallet()), + }; + expect(isHardwareWalletLockedSelector(state)).toBe(false); + }); +}); diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts index 9f452cbc6c..6ac0ad1320 100644 --- a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -1,6 +1,26 @@ import { loadSettings } from "../handlers/loadSettings"; import { saveSettings } from "../handlers/saveSettings"; -import { IS_OPEN_SIDEBAR_BY_DEFAULT_ID } from "constants/localStorageTypes"; +import { + AUTO_LOCK_TIMEOUT_MINUTES_ID, + IS_OPEN_SIDEBAR_BY_DEFAULT_ID, +} from "constants/localStorageTypes"; +import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; +import browser from "webextension-polyfill"; + +const alarmsGet = jest.fn(); +const alarmsCreate = jest.fn().mockResolvedValue(undefined); +const alarmsClear = jest.fn().mockResolvedValue(undefined); + +beforeEach(() => { + alarmsGet.mockReset(); + alarmsCreate.mockClear(); + alarmsClear.mockClear(); + (browser as any).alarms = { + get: alarmsGet, + create: alarmsCreate, + clear: alarmsClear, + }; +}); jest.mock("background/helpers/account", () => ({ getAllowList: jest.fn().mockResolvedValue([]), @@ -10,6 +30,7 @@ jest.mock("background/helpers/account", () => ({ getIsHideDustEnabled: jest.fn().mockResolvedValue(true), getIsMemoValidationEnabled: jest.fn().mockResolvedValue(true), getIsNonSSLEnabled: jest.fn().mockResolvedValue(false), + getIsHardwareWalletActive: jest.fn().mockResolvedValue(false), getNetworkDetails: jest.fn().mockResolvedValue({ network: "TESTNET", networkName: "Test Net", @@ -72,10 +93,25 @@ describe("loadSettings isOpenSidebarByDefault", () => { }); describe("saveSettings isOpenSidebarByDefault", () => { + const makeSessionStore = (hashKey: string | null = null) => + ({ + getState: () => ({ + session: hashKey ? { hashKey: { key: hashKey, salt: "s" } } : {}, + }), + }) as any; + + const makeSessionTimer = () => + ({ + resetSession: jest.fn().mockResolvedValue(undefined), + startSession: jest.fn().mockResolvedValue(undefined), + stopSession: jest.fn().mockResolvedValue(undefined), + }) as any; + it("calls setPanelBehavior with the boolean from the request", async () => { const localStore = { getItem: jest.fn().mockImplementation((key: string) => { if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) return Promise.resolve(true); + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(15); return Promise.resolve(null); }), setItem: jest.fn().mockResolvedValue(undefined), @@ -86,9 +122,15 @@ describe("saveSettings isOpenSidebarByDefault", () => { isMemoValidationEnabled: true, isHideDustEnabled: true, isOpenSidebarByDefault: true, + autoLockTimeoutMinutes: 15, } as any; - const result = await saveSettings({ request, localStore }); + const result = await saveSettings({ + request, + localStore, + sessionStore: makeSessionStore(), + sessionTimer: makeSessionTimer(), + }); expect(chrome.sidePanel.setPanelBehavior).toHaveBeenCalledWith({ openPanelOnActionClick: true, @@ -101,6 +143,7 @@ describe("saveSettings isOpenSidebarByDefault", () => { getItem: jest.fn().mockImplementation((key: string) => { if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) return Promise.resolve(false); + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(15); return Promise.resolve(null); }), setItem: jest.fn().mockResolvedValue(undefined), @@ -111,10 +154,195 @@ describe("saveSettings isOpenSidebarByDefault", () => { isMemoValidationEnabled: true, isHideDustEnabled: true, isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: 15, } as any; - const result = await saveSettings({ request, localStore }); + const result = await saveSettings({ + request, + localStore, + sessionStore: makeSessionStore(), + sessionTimer: makeSessionTimer(), + }); expect(result.isOpenSidebarByDefault).toBe(false); expect(typeof result.isOpenSidebarByDefault).toBe("boolean"); }); }); + +describe("saveSettings autoLockTimeoutMinutes", () => { + const makeSessionStore = (hashKey: string | null = null) => + ({ + getState: () => ({ + session: hashKey ? { hashKey: { key: hashKey, salt: "s" } } : {}, + }), + }) as any; + + const makeSessionTimer = () => + ({ + resetSession: jest.fn().mockResolvedValue(undefined), + startSession: jest.fn().mockResolvedValue(undefined), + stopSession: jest.fn().mockResolvedValue(undefined), + }) as any; + + const makeLocalStore = (storedTimeout: number | null = 15) => + ({ + getItem: jest.fn().mockImplementation((key: string) => { + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) + return Promise.resolve(storedTimeout); + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) + return Promise.resolve(false); + return Promise.resolve(null); + }), + setItem: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + }) as any; + + const baseRequest = { + isDataSharingAllowed: true, + isMemoValidationEnabled: true, + isHideDustEnabled: true, + isOpenSidebarByDefault: false, + }; + + it("rejects invalid autoLockTimeoutMinutes values", async () => { + const localStore = makeLocalStore(); + const result = await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: 7 } as any, + localStore, + sessionStore: makeSessionStore(), + sessionTimer: makeSessionTimer(), + }); + expect((result as any).error).toBe("Invalid autoLockTimeoutMinutes"); + expect(localStore.setItem).not.toHaveBeenCalled(); + }); + + it("rejects non-numeric autoLockTimeoutMinutes", async () => { + const result = await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: "15" } as any, + localStore: makeLocalStore(), + sessionStore: makeSessionStore(), + sessionTimer: makeSessionTimer(), + }); + expect((result as any).error).toBe("Invalid autoLockTimeoutMinutes"); + }); + + it("persists a valid timeout and reschedules when unlocked", async () => { + alarmsGet.mockResolvedValue(undefined); + const localStore = makeLocalStore(30); + const sessionTimer = makeSessionTimer(); + + const result = await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: 30 } as any, + localStore, + sessionStore: makeSessionStore("hash-key"), + sessionTimer, + }); + + expect(localStore.setItem).toHaveBeenCalledWith( + AUTO_LOCK_TIMEOUT_MINUTES_ID, + 30, + ); + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + expect((result as any).autoLockTimeoutMinutes).toBe(30); + expect((result as any).wasLocked).toBe(false); + }); + + it("does not reschedule the timer when the wallet is locked", async () => { + alarmsGet.mockResolvedValue(undefined); + const sessionTimer = makeSessionTimer(); + await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: 5 } as any, + localStore: makeLocalStore(5), + sessionStore: makeSessionStore(null), + sessionTimer, + }); + expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + }); + + it("locks immediately when the new timeout has already elapsed", async () => { + // Previously 60 min, alarm was scheduled to fire in 10 min → 50 min + // already elapsed. User shrinks the timeout to 30 min — that + // threshold has already passed, so we lock now rather than rearm. + alarmsGet.mockResolvedValue({ + scheduledTime: Date.now() + 10 * 60_000, + }); + const localStore = makeLocalStore(60); + const sessionTimer = makeSessionTimer(); + const sessionStore = { + getState: () => ({ session: { hashKey: { key: "k" } } }), + dispatch: jest.fn(), + } as any; + + const result = await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: 30 } as any, + localStore, + sessionStore, + sessionTimer, + }); + + expect(sessionTimer.stopSession).toHaveBeenCalledTimes(1); + expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + expect(sessionStore.dispatch).toHaveBeenCalled(); + expect((result as any).wasLocked).toBe(true); + }); + + it("rearms when the new timeout still has time remaining", async () => { + // Previously 60 min, alarm scheduled to fire in 50 min → 10 min + // already elapsed. User shrinks to 30 min — still within budget, + // so we just rearm at +30. + alarmsGet.mockResolvedValue({ + scheduledTime: Date.now() + 50 * 60_000, + }); + const localStore = makeLocalStore(60); + const sessionTimer = makeSessionTimer(); + + const result = await saveSettings({ + request: { ...baseRequest, autoLockTimeoutMinutes: 30 } as any, + localStore, + sessionStore: makeSessionStore("k"), + sessionTimer, + }); + + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + expect(sessionTimer.stopSession).not.toHaveBeenCalled(); + expect((result as any).wasLocked).toBe(false); + }); +}); + +describe("loadSettings autoLockTimeoutMinutes", () => { + it("returns the stored timeout when valid", async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(30); + return Promise.resolve(null); + }), + setItem: jest.fn(), + } as any; + const result = await loadSettings({ localStore }); + expect(result.autoLockTimeoutMinutes).toBe(30); + }); + + it("falls back to the default when storage is empty", async () => { + const localStore = { + getItem: jest.fn().mockResolvedValue(null), + setItem: jest.fn(), + } as any; + const result = await loadSettings({ localStore }); + expect(result.autoLockTimeoutMinutes).toBe( + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + ); + }); + + it("falls back to the default when storage holds an invalid value", async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(7); + return Promise.resolve(null); + }), + setItem: jest.fn(), + } as any; + const result = await loadSettings({ localStore }); + expect(result.autoLockTimeoutMinutes).toBe( + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + ); + }); +}); diff --git a/extension/src/background/messageListener/__tests__/userActivity.test.ts b/extension/src/background/messageListener/__tests__/userActivity.test.ts new file mode 100644 index 0000000000..2dcadf2e34 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/userActivity.test.ts @@ -0,0 +1,81 @@ +import { userActivity } from "../handlers/userActivity"; + +jest.mock("background/helpers/account", () => ({ + getIsHardwareWalletActive: jest.fn().mockResolvedValue(false), +})); + +const makeSessionStore = (state: any) => + ({ + getState: () => ({ session: state }), + }) as any; + +const makeSessionTimer = () => + ({ + resetSession: jest.fn().mockResolvedValue(undefined), + startSession: jest.fn().mockResolvedValue(undefined), + stopSession: jest.fn().mockResolvedValue(undefined), + }) as any; + +const makeLocalStore = () => + ({ + getItem: jest.fn().mockResolvedValue(null), + setItem: jest.fn(), + remove: jest.fn(), + }) as any; + +describe("userActivity handler", () => { + it("resets the session timer when the wallet is unlocked (hot wallet)", async () => { + const sessionTimer = makeSessionTimer(); + const result = await userActivity({ + sessionStore: makeSessionStore({ hashKey: { key: "deadbeef" } }), + sessionTimer, + localStore: makeLocalStore(), + }); + expect(result).toEqual({ ok: true }); + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + }); + + it("rejects when the wallet is locked", async () => { + const sessionTimer = makeSessionTimer(); + const result = await userActivity({ + sessionStore: makeSessionStore({ hashKey: { key: "" } }), + sessionTimer, + localStore: makeLocalStore(), + }); + expect(result).toEqual({ ok: false }); + expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + }); + + it("rejects when a hardware wallet is active but locked", async () => { + const { getIsHardwareWalletActive } = jest.requireMock( + "background/helpers/account", + ); + (getIsHardwareWalletActive as jest.Mock).mockResolvedValueOnce(true); + const sessionTimer = makeSessionTimer(); + const result = await userActivity({ + sessionStore: makeSessionStore({ + hashKey: { key: "" }, + isHardwareWalletLocked: true, + }), + sessionTimer, + localStore: makeLocalStore(), + }); + expect(result).toEqual({ ok: false }); + expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + }); + + it("resets when a hardware wallet is active and unlocked", async () => { + const { getIsHardwareWalletActive } = jest.requireMock( + "background/helpers/account", + ); + (getIsHardwareWalletActive as jest.Mock).mockResolvedValueOnce(true); + const sessionTimer = makeSessionTimer(); + const result = await userActivity({ + sessionStore: makeSessionStore({ hashKey: { key: "" } }), + sessionTimer, + localStore: makeLocalStore(), + }); + expect(result).toEqual({ ok: true }); + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + }); +}); diff --git a/extension/src/background/messageListener/handlers/confirmPassword.ts b/extension/src/background/messageListener/handlers/confirmPassword.ts index 5c07309f50..f935edc6de 100644 --- a/extension/src/background/messageListener/handlers/confirmPassword.ts +++ b/extension/src/background/messageListener/handlers/confirmPassword.ts @@ -20,6 +20,7 @@ import { allAccountsSelector, buildHasPrivateKeySelector, publicKeySelector, + unlockHardwareWallet, } from "background/ducks/session"; export const confirmPassword = async ({ @@ -66,6 +67,11 @@ export const confirmPassword = async ({ return { error: "Incorrect password" }; } + // A successful password unlock also restores hardware-wallet access + // for mixed-account installs — match today's behavior, where any + // valid password unlock returns the user to a fully unlocked state. + sessionStore.dispatch(unlockHardwareWallet()); + const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); return { publicKey: publicKeySelector(sessionStore.getState()), diff --git a/extension/src/background/messageListener/handlers/createAccount.ts b/extension/src/background/messageListener/handlers/createAccount.ts index f8b85b70c6..680fd93a9c 100644 --- a/extension/src/background/messageListener/handlers/createAccount.ts +++ b/extension/src/background/messageListener/handlers/createAccount.ts @@ -77,7 +77,7 @@ export const createAccount = async ({ const currentState = sessionStore.getState(); - sessionTimer.startSession(); + await sessionTimer.startSession(); const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); return { diff --git a/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts b/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts index 9b18c971f8..32acc81b6a 100644 --- a/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts +++ b/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts @@ -5,15 +5,26 @@ import { } from "@shared/api/types/message-request"; import { captureException } from "@sentry/browser"; -export const handleSignedHwPayload = ({ +import { SessionTimer } from "background/helpers/session"; + +export const handleSignedHwPayload = async ({ request, responseQueue, + sessionTimer, }: { request: HandleSignedHWPayloadMessage; responseQueue: ResponseQueue; + sessionTimer: SessionTimer; }) => { const { signedPayload, uuid } = request; + // A user just completed a hardware-wallet signature — that is a real + // user action, so extend the idle session. Without this the popup + // ping is the only path that refreshes the alarm, and a slow HW + // signing flow could outlast the timeout while the user is actively + // working. + await sessionTimer.resetSession(); + if (!uuid) { captureException("handleSignedHwPayload: missing uuid in request"); return { error: "Transaction not found" }; diff --git a/extension/src/background/messageListener/handlers/loadSettings.ts b/extension/src/background/messageListener/handlers/loadSettings.ts index 5f53b747fb..0751fef751 100644 --- a/extension/src/background/messageListener/handlers/loadSettings.ts +++ b/extension/src/background/messageListener/handlers/loadSettings.ts @@ -12,7 +12,9 @@ import { getOverriddenBlockaidResponse, } from "background/helpers/account"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; +import { coerceAutoLockTimeoutMinutes } from "@shared/constants/autoLock"; import { + AUTO_LOCK_TIMEOUT_MINUTES_ID, DATA_SHARING_ID, IS_OPEN_SIDEBAR_BY_DEFAULT_ID, } from "constants/localStorageTypes"; @@ -34,6 +36,9 @@ export const loadSettings = async ({ const isOpenSidebarByDefault = ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? false; + const autoLockTimeoutMinutes = coerceAutoLockTimeoutMinutes( + await localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID), + ); const { hiddenAssets } = await getHiddenAssets({ localStore }); const overriddenBlockaidResponse = await getOverriddenBlockaidResponse({ localStore, @@ -53,6 +58,7 @@ export const loadSettings = async ({ isNonSSLEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, hiddenAssets, overriddenBlockaidResponse, }; diff --git a/extension/src/background/messageListener/handlers/migrateAccounts.ts b/extension/src/background/messageListener/handlers/migrateAccounts.ts index 81334f1e30..75df3a5e54 100644 --- a/extension/src/background/messageListener/handlers/migrateAccounts.ts +++ b/extension/src/background/messageListener/handlers/migrateAccounts.ts @@ -235,7 +235,7 @@ export const migrateAccounts = async ({ await clearSession({ localStore, sessionStore }); - sessionTimer.startSession(); + await sessionTimer.startSession(); const hashKey = await deriveKeyFromString(password); await storeEncryptedTemporaryData({ localStore, diff --git a/extension/src/background/messageListener/handlers/recoverAccount.ts b/extension/src/background/messageListener/handlers/recoverAccount.ts index c6690c55dc..06a83f9b53 100644 --- a/extension/src/background/messageListener/handlers/recoverAccount.ts +++ b/extension/src/background/messageListener/handlers/recoverAccount.ts @@ -151,7 +151,7 @@ export const recoverAccount = async ({ }); // start the timer now that we have active private key - sessionTimer.startSession(); + await sessionTimer.startSession(); } const currentState = sessionStore.getState(); diff --git a/extension/src/background/messageListener/handlers/saveSettings.ts b/extension/src/background/messageListener/handlers/saveSettings.ts index f89d7a757f..aacf5d1e9d 100644 --- a/extension/src/background/messageListener/handlers/saveSettings.ts +++ b/extension/src/background/messageListener/handlers/saveSettings.ts @@ -1,4 +1,11 @@ +import browser from "webextension-polyfill"; +import { Store } from "redux"; + import { SaveSettingsMessage } from "@shared/api/types/message-request"; +import { + coerceAutoLockTimeoutMinutes, + isValidAutoLockTimeoutMinutes, +} from "@shared/constants/autoLock"; import { getAllowList, getFeatureFlags, @@ -10,6 +17,16 @@ import { } from "background/helpers/account"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { + buildHasPrivateKeySelector, + SessionState, +} from "background/ducks/session"; +import { + clearSession, + SESSION_ALARM_NAME, + SessionTimer, +} from "background/helpers/session"; +import { + AUTO_LOCK_TIMEOUT_MINUTES_ID, DATA_SHARING_ID, IS_HIDE_DUST_ENABLED_ID, IS_OPEN_SIDEBAR_BY_DEFAULT_ID, @@ -19,17 +36,32 @@ import { export const saveSettings = async ({ request, localStore, + sessionStore, + sessionTimer, }: { request: SaveSettingsMessage; localStore: DataStorageAccess; + sessionStore: Store; + sessionTimer: SessionTimer; }) => { const { isDataSharingAllowed, isMemoValidationEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, } = request; + if (!isValidAutoLockTimeoutMinutes(autoLockTimeoutMinutes)) { + return { error: "Invalid autoLockTimeoutMinutes" }; + } + + // Capture the previous timeout *before* writing the new one so we can + // reason about elapsed-idle time against the alarm currently in flight. + const previousAutoLockTimeoutMinutes = coerceAutoLockTimeoutMinutes( + await localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID), + ); + await localStore.setItem(DATA_SHARING_ID, isDataSharingAllowed); await localStore.setItem(IS_VALIDATING_MEMO_ID, isMemoValidationEnabled); await localStore.setItem(IS_HIDE_DUST_ENABLED_ID, isHideDustEnabled); @@ -37,6 +69,44 @@ export const saveSettings = async ({ IS_OPEN_SIDEBAR_BY_DEFAULT_ID, isOpenSidebarByDefault, ); + await localStore.setItem( + AUTO_LOCK_TIMEOUT_MINUTES_ID, + autoLockTimeoutMinutes, + ); + + // A new auto-lock timeout takes effect immediately, but only if the + // wallet is currently unlocked. When shortening the timeout, the user + // may already have been idle longer than the new threshold — in that + // case we lock immediately rather than schedule an alarm in the past. + // `wasLocked` is propagated to the popup so its `auth.hasPrivateKey` + // can flip without waiting for the next `useGetAppData` poll. + let wasLocked = false; + const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); + const isUnlocked = await hasPrivateKeySelector( + sessionStore.getState() as SessionState, + ); + if (isUnlocked) { + const existingAlarm = await browser.alarms.get(SESSION_ALARM_NAME); + if (!existingAlarm) { + // Recovery edge: the worker just woke and the alarm hasn't been + // re-observed yet (or this is the first save after unlock). + // Don't synthesize an immediate lock; just rearm with the new + // timeout. + await sessionTimer.resetSession(); + } else { + const newDelayMs = autoLockTimeoutMinutes * 60_000; + const oldDelayMs = previousAutoLockTimeoutMinutes * 60_000; + const remainingMs = existingAlarm.scheduledTime - Date.now(); + const elapsedIdleMs = Math.max(0, oldDelayMs - remainingMs); + if (elapsedIdleMs >= newDelayMs) { + await clearSession({ sessionStore, localStore }); + await sessionTimer.stopSession(); + wasLocked = true; + } else { + await sessionTimer.resetSession(); + } + } + } // Apply sidebar behavior immediately on Chrome if (chrome.sidePanel?.setPanelBehavior) { @@ -62,5 +132,9 @@ export const saveSettings = async ({ isOpenSidebarByDefault: ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? false, + autoLockTimeoutMinutes: coerceAutoLockTimeoutMinutes( + await localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID), + ), + wasLocked, }; }; diff --git a/extension/src/background/messageListener/handlers/signOut.ts b/extension/src/background/messageListener/handlers/signOut.ts index 96959d9989..410a7ece6b 100644 --- a/extension/src/background/messageListener/handlers/signOut.ts +++ b/extension/src/background/messageListener/handlers/signOut.ts @@ -2,6 +2,7 @@ import { Store } from "redux"; import { logOut, publicKeySelector } from "background/ducks/session"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; +import { SessionTimer } from "background/helpers/session"; import { APPLICATION_ID, TEMPORARY_STORE_ID, @@ -10,12 +11,17 @@ import { export const signOut = async ({ localStore, sessionStore, + sessionTimer, }: { localStore: DataStorageAccess; sessionStore: Store; + sessionTimer: SessionTimer; }) => { sessionStore.dispatch(logOut()); await localStore.remove(TEMPORARY_STORE_ID); + // Cancel any pending auto-lock alarm — the wallet is being locked + // explicitly, so the idle timer no longer needs to fire. + await sessionTimer.stopSession(); return { publicKey: publicKeySelector(sessionStore.getState()), diff --git a/extension/src/background/messageListener/handlers/userActivity.ts b/extension/src/background/messageListener/handlers/userActivity.ts new file mode 100644 index 0000000000..1c6080e57b --- /dev/null +++ b/extension/src/background/messageListener/handlers/userActivity.ts @@ -0,0 +1,46 @@ +import { Store } from "redux"; + +import { + buildHasPrivateKeySelector, + SessionState, +} from "background/ducks/session"; +import { SessionTimer } from "background/helpers/session"; +import { DataStorageAccess } from "background/helpers/dataStorageAccess"; + +/** + * Handle a USER_ACTIVITY ping from an extension page. + * + * When the wallet is unlocked, this rearms the idle auto-lock alarm + * via `sessionTimer.resetSession()`. "Unlocked" means either the + * hot-wallet `hashKey` is present OR a hardware wallet is active and + * not idle-locked — `buildHasPrivateKeySelector` is the canonical + * predicate, used by the popup router for the same purpose. + * + * When locked we reject the ping — a stale activity listener in a + * still-mounted extension page must not re-arm the alarm after the + * user has signed out or auto-locked elsewhere. + * + * Caller-side, `popupMessageListener` additionally gates this message + * behind `isFromExtensionPage` so dApp content scripts cannot extend + * an unlocked session. + */ +export const userActivity = async ({ + sessionStore, + sessionTimer, + localStore, +}: { + sessionStore: Store; + sessionTimer: SessionTimer; + localStore: DataStorageAccess; +}) => { + const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); + const isUnlocked = await hasPrivateKeySelector( + sessionStore.getState() as SessionState, + ); + if (!isUnlocked) { + return { ok: false }; + } + + await sessionTimer.resetSession(); + return { ok: true }; +}; diff --git a/extension/src/background/messageListener/helpers/login-all-accounts.ts b/extension/src/background/messageListener/helpers/login-all-accounts.ts index 4a81587e1a..89181a614f 100644 --- a/extension/src/background/messageListener/helpers/login-all-accounts.ts +++ b/extension/src/background/messageListener/helpers/login-all-accounts.ts @@ -135,5 +135,5 @@ export const loginToAllAccounts = async ( } // start the timer now that we have active private key - sessionTimer.startSession(); + await sessionTimer.startSession(); }; diff --git a/extension/src/background/messageListener/helpers/test-helpers.ts b/extension/src/background/messageListener/helpers/test-helpers.ts index 948510ed3e..f4e4045257 100644 --- a/extension/src/background/messageListener/helpers/test-helpers.ts +++ b/extension/src/background/messageListener/helpers/test-helpers.ts @@ -8,6 +8,11 @@ import { sessionSlice } from "background/ducks/session"; import { dataStorageAccess } from "background/helpers/dataStorageAccess"; import { combineReducers } from "redux"; import { configureStore } from "@reduxjs/toolkit"; +import { + coerceAutoLockTimeoutMinutes, + AutoLockTimeoutMinutes, +} from "@shared/constants/autoLock"; +import { AUTO_LOCK_TIMEOUT_MINUTES_ID } from "constants/localStorageTypes"; const mockStore: Record = {}; const mockStorageApi = { @@ -81,18 +86,36 @@ const mockKeyManager = new KeyManager({ mockKeyManager.registerEncrypter(ScryptEncrypter); const MOCK_TIMER_DURATION = 60 * 24; + +/** + * Test double for `SessionTimer`. Reads the auto-lock timeout from the + * mock storage on each `resetSession()` / `startSession()` call so + * tests can simulate settings changes via `mockStore`. Tracks the + * number of resets/stops to allow assertions on alarm behavior. + */ class MockBrowserAlarm { duration = 1000 * 60 * MOCK_TIMER_DURATION; runningTimeout: null | ReturnType = null; callback: () => void; + resetCount = 0; + stopCount = 0; + lastTimeoutMinutes: AutoLockTimeoutMinutes | null = null; constructor(callback: () => unknown, duration?: number) { this.duration = duration || this.duration; this.callback = callback; } - startSession() { - this.duration = 1000 * 60 * MOCK_TIMER_DURATION; + async startSession() { + await this.resetSession(); + } + + async resetSession() { + const stored = mockStore[AUTO_LOCK_TIMEOUT_MINUTES_ID]; + const minutes = coerceAutoLockTimeoutMinutes(stored); + this.lastTimeoutMinutes = minutes; + this.duration = 1000 * 60 * minutes; + this.resetCount += 1; if (this.runningTimeout) clearTimeout(this.runningTimeout); this.runningTimeout = setTimeout(() => { @@ -101,6 +124,14 @@ class MockBrowserAlarm { } }, this.duration); } + + async stopSession() { + this.stopCount += 1; + if (this.runningTimeout) { + clearTimeout(this.runningTimeout); + this.runningTimeout = null; + } + } } export { diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 4d1bc3fed9..49d1441c5e 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -64,6 +64,7 @@ import { loadLastUsedAccount } from "./handlers/loadLastAccountUsed"; import { signOut } from "./handlers/signOut"; import { saveAllowList } from "./handlers/saveAllowList"; import { saveSettings } from "./handlers/saveSettings"; +import { userActivity } from "./handlers/userActivity"; import { saveExperimentalFeatures } from "./handlers/saveExperimentalFeatures"; import { loadSettings } from "./handlers/loadSettings"; import { getCachedAssetIconList } from "./handlers/getCachedAssetIconList"; @@ -311,6 +312,7 @@ export const popupMessageListener = ( return handleSignedHwPayload({ request, responseQueue, + sessionTimer, }); } case SERVICE_TYPES.ADD_TOKEN: { @@ -401,6 +403,7 @@ export const popupMessageListener = ( return signOut({ localStore, sessionStore, + sessionTimer, }); } case SERVICE_TYPES.SAVE_ALLOWLIST: { @@ -414,6 +417,8 @@ export const popupMessageListener = ( return saveSettings({ request, localStore, + sessionStore, + sessionTimer, }); } case SERVICE_TYPES.SAVE_EXPERIMENTAL_FEATURES: { @@ -624,6 +629,11 @@ export const popupMessageListener = ( })(); } + case SERVICE_TYPES.USER_ACTIVITY: { + if (!isFromExtensionPage) return { error: "Unauthorized" }; + return userActivity({ sessionStore, sessionTimer, localStore }); + } + default: return { error: "Message type not supported" }; } diff --git a/extension/src/constants/localStorageTypes.ts b/extension/src/constants/localStorageTypes.ts index b24419877b..764180f147 100644 --- a/extension/src/constants/localStorageTypes.ts +++ b/extension/src/constants/localStorageTypes.ts @@ -34,3 +34,4 @@ export const IS_OPEN_SIDEBAR_BY_DEFAULT_ID = "isOpenSidebarByDefault"; export const METRICS_USER_ID = "metrics_user_id"; export const RECENT_PROTOCOLS = "recentProtocols"; export const HAS_SEEN_DISCOVER_WELCOME = "hasSeenDiscoverWelcome"; +export const AUTO_LOCK_TIMEOUT_MINUTES_ID = "autoLockTimeoutMinutes"; diff --git a/extension/src/popup/App.tsx b/extension/src/popup/App.tsx index c92ba1b1c1..02acb36279 100755 --- a/extension/src/popup/App.tsx +++ b/extension/src/popup/App.tsx @@ -15,6 +15,7 @@ import { reducer as cache } from "popup/ducks/cache"; import { reducer as remoteConfig } from "popup/ducks/remoteConfig"; import { ErrorTracking } from "popup/components/ErrorTracking"; import { AccountMismatch } from "popup/components/AccountMismatch"; +import { ActivityTracker } from "popup/components/ActivityTracker"; import { MaintenanceScreen } from "popup/components/MaintenanceScreen"; import { useRemoteConfig } from "popup/helpers/hooks/useRemoteConfig"; import { maintenanceScreenSelector } from "popup/ducks/remoteConfig"; @@ -66,6 +67,7 @@ export const App = () => ( + diff --git a/extension/src/popup/components/ActivityTracker/index.tsx b/extension/src/popup/components/ActivityTracker/index.tsx new file mode 100644 index 0000000000..a3ce98a991 --- /dev/null +++ b/extension/src/popup/components/ActivityTracker/index.tsx @@ -0,0 +1,17 @@ +import { useSelector } from "react-redux"; + +import { hasPrivateKeySelector } from "popup/ducks/accountServices"; +import { useActivityPing } from "popup/helpers/hooks/useActivityPing"; + +/** + * Thin redux-aware wrapper that drives the idle activity ping. Lives + * inside `` (mounted by `popup/App.tsx`) so it has access to + * `hasPrivateKeySelector`, which is the canonical "wallet is unlocked" + * signal across popup, sidebar, and standalone signing/grant-access + * windows — all of which mount the same App. + */ +export const ActivityTracker = () => { + const isUnlocked = useSelector(hasPrivateKeySelector); + useActivityPing(isUnlocked); + return null; +}; diff --git a/extension/src/popup/ducks/__tests__/settings.test.ts b/extension/src/popup/ducks/__tests__/settings.test.ts new file mode 100644 index 0000000000..b4862957ba --- /dev/null +++ b/extension/src/popup/ducks/__tests__/settings.test.ts @@ -0,0 +1,92 @@ +import { combineReducers, configureStore } from "@reduxjs/toolkit"; + +import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; +import { saveSettings as saveSettingsService } from "@shared/api/internal"; +import { SettingsState } from "@shared/api/types"; +import { reducer as authReducer } from "../accountServices"; +import { reducer as settingsReducer, saveSettings } from "../settings"; + +jest.mock("@shared/api/internal", () => ({ + ...jest.requireActual("@shared/api/internal"), + saveSettings: jest.fn(), +})); + +const makeSettingsResponse = (wasLocked?: boolean) => ({ + allowList: {}, + isDataSharingAllowed: true, + isMemoValidationEnabled: true, + isHideDustEnabled: true, + isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + networkDetails: { + network: "TESTNET", + networkName: "Testnet", + networkUrl: "https://horizon-testnet.stellar.org", + networkPassphrase: "Test SDF Network ; September 2015", + }, + networksList: [], + isRpcHealthy: true, + isSorobanPublicEnabled: false, + settingsState: SettingsState.SUCCESS, + userNotification: { enabled: false, message: "" }, + wasLocked, +}); + +const makeStore = () => + configureStore({ + reducer: combineReducers({ auth: authReducer, settings: settingsReducer }), + preloadedState: { + auth: { + allAccounts: [], + migratedAccounts: [], + applicationState: "APPLICATION_STARTED", + hasPrivateKey: true, + publicKey: "GBTEST", + connectingWalletType: "NONE", + bipPath: "", + tokenIdList: [], + error: "", + accountStatus: "IDLE", + isAccountMismatch: false, + }, + } as any, + }); + +const request = { + isDataSharingAllowed: true, + isMemoValidationEnabled: true, + isHideDustEnabled: true, + isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, +}; + +describe("settings saveSettings thunk", () => { + beforeEach(() => { + (saveSettingsService as jest.Mock).mockReset(); + }); + + it("dispatches lockAccount when the background reports an immediate lock", async () => { + (saveSettingsService as jest.Mock).mockResolvedValue( + makeSettingsResponse(true), + ); + const store = makeStore(); + + await store.dispatch(saveSettings(request) as any); + + expect(store.getState().auth.hasPrivateKey).toBe(false); + }); + + it.each([false, undefined])( + "does not lock the popup auth state when wasLocked is %s", + async (wasLocked) => { + (saveSettingsService as jest.Mock).mockResolvedValue( + makeSettingsResponse(wasLocked), + ); + const store = makeStore(); + + await store.dispatch(saveSettings(request) as any); + + expect(store.getState().auth.hasPrivateKey).toBe(true); + }, + ); +}); diff --git a/extension/src/popup/ducks/accountServices.ts b/extension/src/popup/ducks/accountServices.ts index a397ebafd6..920e94bfea 100644 --- a/extension/src/popup/ducks/accountServices.ts +++ b/extension/src/popup/ducks/accountServices.ts @@ -570,6 +570,9 @@ const authSlice = createSlice({ clearApiError(state) { state.error = ""; }, + lockAccount(state) { + state.hasPrivateKey = false; + }, setConnectingWalletType(state, action) { state.connectingWalletType = action.payload; }, @@ -990,6 +993,7 @@ export const { saveAccount, saveAccountError, saveApplicationState, + lockAccount, } = authSlice.actions; export { reducer }; diff --git a/extension/src/popup/ducks/settings.ts b/extension/src/popup/ducks/settings.ts index 7da17788dc..9f8fabfe92 100644 --- a/extension/src/popup/ducks/settings.ts +++ b/extension/src/popup/ducks/settings.ts @@ -28,6 +28,10 @@ import { AssetsLists, DEFAULT_ASSETS_LISTS, } from "@shared/constants/soroban/asset-list"; +import { + AutoLockTimeoutMinutes, + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, +} from "@shared/constants/autoLock"; import { AllowList, @@ -36,7 +40,7 @@ import { SettingsState, ExperimentalFeatures, } from "@shared/api/types"; -import { publicKeySelector } from "popup/ducks/accountServices"; +import { lockAccount, publicKeySelector } from "popup/ducks/accountServices"; import { AppState } from "popup/App"; import { isMainnet } from "helpers/stellar"; @@ -59,6 +63,8 @@ const settingsInitialState: Settings = { isMemoValidationEnabled: true, isHideDustEnabled: true, isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES as AutoLockTimeoutMinutes, error: "", }; @@ -120,12 +126,13 @@ export const saveAllowList = createAsyncThunk< ); export const saveSettings = createAsyncThunk< - Settings & IndexerSettings, + Settings & IndexerSettings & { wasLocked?: boolean }, { isDataSharingAllowed: boolean; isMemoValidationEnabled: boolean; isHideDustEnabled: boolean; isOpenSidebarByDefault: boolean; + autoLockTimeoutMinutes: AutoLockTimeoutMinutes; }, { rejectValue: ErrorMessage; state: AppState } >( @@ -136,10 +143,11 @@ export const saveSettings = createAsyncThunk< isMemoValidationEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, }, - { getState, rejectWithValue }, + { dispatch, getState, rejectWithValue }, ) => { - let res = { + let res: Settings & IndexerSettings & { wasLocked?: boolean } = { ...settingsInitialState, isSorobanPublicEnabled: false, isRpcHealthy: false, @@ -147,6 +155,7 @@ export const saveSettings = createAsyncThunk< settingsState: SettingsState.IDLE, isHideDustEnabled: true, isOpenSidebarByDefault: false, + wasLocked: false, }; const activePublicKey = publicKeySelector(getState()); @@ -157,6 +166,7 @@ export const saveSettings = createAsyncThunk< isMemoValidationEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, }); } catch (e) { console.error(e); @@ -166,6 +176,15 @@ export const saveSettings = createAsyncThunk< }); } + // When the background locks the session because the shortened + // timeout has already elapsed, flip the popup's auth slice + // immediately rather than waiting for the next `useGetAppData` + // poll — otherwise the user briefly sees an unlocked UI after + // saving. + if (res.wasLocked) { + dispatch(lockAccount()); + } + return res; }, ); @@ -375,6 +394,7 @@ const settingsSlice = createSlice({ isNonSSLEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, } = payload; state.allowList = allowList; state.isDataSharingAllowed = isDataSharingAllowed; @@ -387,6 +407,8 @@ const settingsSlice = createSlice({ state.isNonSSLEnabled = isNonSSLEnabled; state.isHideDustEnabled = isHideDustEnabled; state.isOpenSidebarByDefault = isOpenSidebarByDefault; + state.autoLockTimeoutMinutes = + autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES; state.overriddenBlockaidResponse = payload.overriddenBlockaidResponse ?? null; state.settingsState = SettingsState.SUCCESS; @@ -436,10 +458,12 @@ const settingsSlice = createSlice({ isSorobanPublicEnabled, isHideDustEnabled, isOpenSidebarByDefault, + autoLockTimeoutMinutes, overriddenBlockaidResponse, } = (action?.payload as typeof action.payload & { overriddenBlockaidResponse?: string | null; isOpenSidebarByDefault?: boolean; + autoLockTimeoutMinutes?: AutoLockTimeoutMinutes; }) || { ...initialState, }; @@ -454,6 +478,8 @@ const settingsSlice = createSlice({ isSorobanPublicEnabled, isHideDustEnabled, isOpenSidebarByDefault: isOpenSidebarByDefault ?? false, + autoLockTimeoutMinutes: + autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, overriddenBlockaidResponse: overriddenBlockaidResponse ?? null, }; }); @@ -700,3 +726,9 @@ export const isOpenSidebarByDefaultSelector = createSelector( settingsSelector, (settings) => settings.isOpenSidebarByDefault, ); + +export const autoLockTimeoutMinutesSelector = createSelector( + settingsSelector, + (settings): AutoLockTimeoutMinutes => + settings.autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, +); diff --git a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts new file mode 100644 index 0000000000..ccfde5d468 --- /dev/null +++ b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts @@ -0,0 +1,97 @@ +import { renderHook } from "@testing-library/react"; + +import { sendMessageToBackground } from "@shared/api/helpers/extensionMessaging"; +import { SERVICE_TYPES } from "@shared/constants/services"; +import { useActivityPing } from "../useActivityPing"; + +jest.mock("@shared/api/helpers/extensionMessaging", () => ({ + sendMessageToBackground: jest.fn().mockResolvedValue({}), +})); + +const activityEvents = ["mousedown", "keydown", "touchstart", "wheel"]; + +describe("useActivityPing", () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(10_000); + (sendMessageToBackground as jest.Mock).mockClear(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("sends USER_ACTIVITY on the first event and throttles events within 5 seconds", () => { + renderHook(() => useActivityPing(true)); + + window.dispatchEvent(new MouseEvent("mousedown")); + window.dispatchEvent(new KeyboardEvent("keydown")); + + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + expect(sendMessageToBackground).toHaveBeenCalledWith({ + type: SERVICE_TYPES.USER_ACTIVITY, + activePublicKey: null, + }); + }); + + it("sends another ping after the 5 second throttle window", () => { + renderHook(() => useActivityPing(true)); + + window.dispatchEvent(new MouseEvent("mousedown")); + jest.setSystemTime(14_999); + window.dispatchEvent(new WheelEvent("wheel")); + jest.setSystemTime(15_000); + window.dispatchEvent(new WheelEvent("wheel")); + + expect(sendMessageToBackground).toHaveBeenCalledTimes(2); + }); + + it("listens to all configured activity events", () => { + renderHook(() => useActivityPing(true)); + + for (const eventName of activityEvents) { + (sendMessageToBackground as jest.Mock).mockClear(); + jest.setSystemTime(Date.now() + 5_000); + window.dispatchEvent(new Event(eventName)); + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + } + }); + + it("cleans up listeners on unmount", () => { + const addSpy = jest.spyOn(window, "addEventListener"); + const removeSpy = jest.spyOn(window, "removeEventListener"); + + const { unmount } = renderHook(() => useActivityPing(true)); + unmount(); + + for (const eventName of activityEvents) { + expect(addSpy).toHaveBeenCalledWith( + eventName, + expect.any(Function), + { passive: true }, + ); + expect(removeSpy).toHaveBeenCalledWith(eventName, expect.any(Function)); + } + + addSpy.mockRestore(); + removeSpy.mockRestore(); + }); + + it("does not register listeners when locked", () => { + const addSpy = jest.spyOn(window, "addEventListener"); + + renderHook(() => useActivityPing(false)); + window.dispatchEvent(new MouseEvent("mousedown")); + + for (const eventName of activityEvents) { + expect(addSpy).not.toHaveBeenCalledWith( + eventName, + expect.any(Function), + expect.anything(), + ); + } + expect(sendMessageToBackground).not.toHaveBeenCalled(); + + addSpy.mockRestore(); + }); +}); diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts new file mode 100644 index 0000000000..bbc86ebfdb --- /dev/null +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -0,0 +1,51 @@ +import { useEffect } from "react"; + +import { sendMessageToBackground } from "@shared/api/helpers/extensionMessaging"; +import { SERVICE_TYPES } from "@shared/constants/services"; + +const PING_THROTTLE_MS = 5_000; + +// We deliberately do NOT listen for `scroll`: Freighter's layouts use +// inner overflow containers (e.g. `popup/basics/layout/View`) where +// `scroll` does not bubble to `window`. `wheel` does bubble for mouse +// scrolling, and pointer/keyboard-driven scrolling already fires +// `mousedown` / `keydown`. +const ACTIVITY_EVENTS = [ + "mousedown", + "keydown", + "touchstart", + "wheel", +] as const; + +/** + * Ping the background with USER_ACTIVITY messages whenever the user + * interacts with the extension page, while the wallet is unlocked. + * Uses a leading-edge throttle so a single ping fires on the first + * event in each `PING_THROTTLE_MS` window — accurate to ~8 % on the + * 1-minute preset and effectively noise at higher presets. + */ +export const useActivityPing = (isUnlocked: boolean) => { + useEffect(() => { + if (!isUnlocked) return undefined; + + let lastPingAt = 0; + const handler = () => { + const now = Date.now(); + if (now - lastPingAt < PING_THROTTLE_MS) return; + lastPingAt = now; + void sendMessageToBackground({ + type: SERVICE_TYPES.USER_ACTIVITY, + activePublicKey: null, + }); + }; + + for (const evt of ACTIVITY_EVENTS) { + window.addEventListener(evt, handler, { passive: true }); + } + return () => { + for (const evt of ACTIVITY_EVENTS) { + window.removeEventListener(evt, handler); + } + }; + }, [isUnlocked]); +}; diff --git a/extension/src/popup/views/IntegrationTest.tsx b/extension/src/popup/views/IntegrationTest.tsx index 9e1c1fa2ab..28e19a7f2f 100644 --- a/extension/src/popup/views/IntegrationTest.tsx +++ b/extension/src/popup/views/IntegrationTest.tsx @@ -2,6 +2,8 @@ import React, { useState, useEffect } from "react"; import { Networks } from "stellar-sdk"; import { useTranslation } from "react-i18next"; +import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; + import { createAccount, changeNetwork, @@ -348,6 +350,7 @@ export const IntegrationTest = () => { isMemoValidationEnabled: true, isHideDustEnabled: true, isOpenSidebarByDefault: false, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }); runAsserts("saveSettings", () => { assertEq(res.networkDetails, FUTURENET_NETWORK_DETAILS); diff --git a/extension/src/popup/views/Preferences/index.tsx b/extension/src/popup/views/Preferences/index.tsx index 821e6e6479..36270fddee 100644 --- a/extension/src/popup/views/Preferences/index.tsx +++ b/extension/src/popup/views/Preferences/index.tsx @@ -1,5 +1,5 @@ import React, { useEffect } from "react"; -import { Notification, Toggle } from "@stellar/design-system"; +import { Notification, Select, Toggle } from "@stellar/design-system"; import { Field, Form, Formik } from "formik"; import { useDispatch } from "react-redux"; import { useTranslation } from "react-i18next"; @@ -16,9 +16,25 @@ import { openTab } from "popup/helpers/navigate"; import { newTabHref } from "helpers/urls"; import { Navigate, useLocation } from "react-router-dom"; import { reRouteOnboarding } from "popup/helpers/route"; +import { + AutoLockTimeoutMinutes, + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + VALID_AUTO_LOCK_TIMEOUT_MINUTES, + coerceAutoLockTimeoutMinutes, +} from "@shared/constants/autoLock"; import "./styles.scss"; +const formatTimeoutLabel = ( + minutes: AutoLockTimeoutMinutes, + t: (key: string, opts?: Record) => string, +) => { + if (minutes >= 60) { + return t("{{count}} hour", { count: minutes / 60 }); + } + return t("{{count}} minute", { count: minutes }); +}; + export const Preferences = () => { const { t } = useTranslation(); const location = useLocation(); @@ -30,6 +46,7 @@ export const Preferences = () => { isDataSharingAllowedValue: boolean; isHideDustEnabledValue: boolean; isOpenSidebarByDefaultValue: boolean; + autoLockTimeoutMinutesValue: AutoLockTimeoutMinutes; } const handleSubmit = async (formValue: SettingValues) => { @@ -38,6 +55,7 @@ export const Preferences = () => { isDataSharingAllowedValue, isHideDustEnabledValue, isOpenSidebarByDefaultValue, + autoLockTimeoutMinutesValue, } = formValue; await dispatch( @@ -46,6 +64,10 @@ export const Preferences = () => { isDataSharingAllowed: isDataSharingAllowedValue, isHideDustEnabled: isHideDustEnabledValue, isOpenSidebarByDefault: isOpenSidebarByDefaultValue, + autoLockTimeoutMinutes: coerceAutoLockTimeoutMinutes( + // Form values from ) and make the conversion in handleSubmit explicit rather than relying on the field type being narrower than reality. - useActivityPing: send activePublicKey as an empty string instead of null to match BaseMessage typing. The background's mismatch check (`request.activePublicKey && …`) treats empty-string exactly like the previous null (skip the check). - useActivityPing test: update expectation to match the empty string. - Prettier: reflow newly added autoLockTimeoutMinutes fields in SignTransaction.test.tsx / Account.test.tsx test fixtures (and three other lightly drifted spots prettier picked up at the same time). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../helpers/__tests__/session.test.ts | 8 ++--- extension/src/background/index.ts | 4 +-- .../__tests__/handleSignedHwPayload.test.ts | 4 ++- .../hooks/__tests__/useActivityPing.test.ts | 10 +++---- .../popup/helpers/hooks/useActivityPing.ts | 9 +++++- .../src/popup/views/Preferences/index.tsx | 30 +++++++++++++++---- .../popup/views/__tests__/Account.test.tsx | 12 ++++---- .../views/__tests__/SignTransaction.test.tsx | 22 +++++++------- 8 files changed, 60 insertions(+), 39 deletions(-) diff --git a/extension/src/background/helpers/__tests__/session.test.ts b/extension/src/background/helpers/__tests__/session.test.ts index 5b281aa1bd..f57a9876db 100644 --- a/extension/src/background/helpers/__tests__/session.test.ts +++ b/extension/src/background/helpers/__tests__/session.test.ts @@ -104,7 +104,8 @@ describe("SessionTimer", () => { const makeLocalStore = (stored: unknown) => ({ getItem: jest.fn().mockImplementation((key: string) => { - if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(stored); + if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) + return Promise.resolve(stored); return Promise.resolve(null); }), setItem: jest.fn(), @@ -145,10 +146,7 @@ describe("SessionTimer", () => { it("re-reads the persisted timeout on every reset", async () => { const localStore = { - getItem: jest - .fn() - .mockResolvedValueOnce(15) - .mockResolvedValueOnce(60), + getItem: jest.fn().mockResolvedValueOnce(15).mockResolvedValueOnce(60), setItem: jest.fn(), remove: jest.fn(), } as any; diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 991838b940..fddcd38f18 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -53,9 +53,7 @@ import { } from "@stellar/typescript-wallet-sdk-km"; import { BrowserStorageConfigParams } from "@stellar/typescript-wallet-sdk-km/lib/Plugins/BrowserStorageFacade"; -const sessionTimer = new SessionTimer( - dataStorageAccess(browserLocalStorage), -); +const sessionTimer = new SessionTimer(dataStorageAccess(browserLocalStorage)); export const initContentScriptMessageListener = () => { browser?.runtime?.onMessage?.addListener((message) => { diff --git a/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts b/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts index 8f3d4561e4..777ca331b7 100644 --- a/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts +++ b/extension/src/background/messageListener/__tests__/handleSignedHwPayload.test.ts @@ -59,7 +59,9 @@ describe("handleSignedHwPayload", () => { let state = { session: sessionSlice.reducer(undefined, { type: "init" }) }; expect(isHardwareWalletLockedSelector(state)).toBe(false); - state = { session: sessionSlice.reducer(state.session, lockHardwareWallet()) }; + state = { + session: sessionSlice.reducer(state.session, lockHardwareWallet()), + }; expect(isHardwareWalletLockedSelector(state)).toBe(true); state = { diff --git a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts index ccfde5d468..b140d76d61 100644 --- a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts +++ b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts @@ -30,7 +30,7 @@ describe("useActivityPing", () => { expect(sendMessageToBackground).toHaveBeenCalledTimes(1); expect(sendMessageToBackground).toHaveBeenCalledWith({ type: SERVICE_TYPES.USER_ACTIVITY, - activePublicKey: null, + activePublicKey: "", }); }); @@ -65,11 +65,9 @@ describe("useActivityPing", () => { unmount(); for (const eventName of activityEvents) { - expect(addSpy).toHaveBeenCalledWith( - eventName, - expect.any(Function), - { passive: true }, - ); + expect(addSpy).toHaveBeenCalledWith(eventName, expect.any(Function), { + passive: true, + }); expect(removeSpy).toHaveBeenCalledWith(eventName, expect.any(Function)); } diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index bbc86ebfdb..b88d2ea3e7 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -35,7 +35,14 @@ export const useActivityPing = (isUnlocked: boolean) => { lastPingAt = now; void sendMessageToBackground({ type: SERVICE_TYPES.USER_ACTIVITY, - activePublicKey: null, + // `USER_ACTIVITY` is account-agnostic — the background only + // cares that *some* extension page is active, not which key is + // selected. Send an empty string (rather than null) to match + // `BaseMessage`'s `activePublicKey: string` typing; the + // background's mismatch check (`if (request.activePublicKey + // && …)`) treats empty-string as "skip the check", same as + // the previous null. + activePublicKey: "", }); }; diff --git a/extension/src/popup/views/Preferences/index.tsx b/extension/src/popup/views/Preferences/index.tsx index 36270fddee..516603c7ce 100644 --- a/extension/src/popup/views/Preferences/index.tsx +++ b/extension/src/popup/views/Preferences/index.tsx @@ -25,14 +25,29 @@ import { import "./styles.scss"; +// Build a human-readable label for a timeout preset. We construct an +// English fallback in JS first and hand it to `t()` as `defaultValue`, +// so that — even when an i18n key is missing (common in dev or partial +// locales) — the rendered label is grammatically correct (e.g. "1 +// minute" / "5 minutes" rather than "1 minute" / "5 minute"). Locales +// remain free to override by providing the matching keys. const formatTimeoutLabel = ( minutes: AutoLockTimeoutMinutes, t: (key: string, opts?: Record) => string, ) => { if (minutes >= 60) { - return t("{{count}} hour", { count: minutes / 60 }); + const hours = minutes / 60; + const fallback = hours === 1 ? "1 hour" : `${hours} hours`; + return t("autoLockTimeout.hours", { + count: hours, + defaultValue: fallback, + }); } - return t("{{count}} minute", { count: minutes }); + const fallback = minutes === 1 ? "1 minute" : `${minutes} minutes`; + return t("autoLockTimeout.minutes", { + count: minutes, + defaultValue: fallback, + }); }; export const Preferences = () => { @@ -46,7 +61,9 @@ export const Preferences = () => { isDataSharingAllowedValue: boolean; isHideDustEnabledValue: boolean; isOpenSidebarByDefaultValue: boolean; - autoLockTimeoutMinutesValue: AutoLockTimeoutMinutes; + // Formik/` arrive as strings; normalize. Number(autoLockTimeoutMinutesValue), ), }), @@ -133,8 +149,10 @@ export const Preferences = () => { isDataSharingAllowedValue: isDataSharingAllowed, isHideDustEnabledValue: isHideDustEnabled, isOpenSidebarByDefaultValue: isOpenSidebarByDefault ?? false, - autoLockTimeoutMinutesValue: coerceAutoLockTimeoutMinutes( - autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutesValue: String( + coerceAutoLockTimeoutMinutes( + autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + ), ), }; diff --git a/extension/src/popup/views/__tests__/Account.test.tsx b/extension/src/popup/views/__tests__/Account.test.tsx index dba59debc1..a0c4fc57ad 100644 --- a/extension/src/popup/views/__tests__/Account.test.tsx +++ b/extension/src/popup/views/__tests__/Account.test.tsx @@ -890,7 +890,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); @@ -945,7 +945,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); jest @@ -1030,7 +1030,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); jest @@ -1114,7 +1114,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); jest @@ -1197,7 +1197,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); jest @@ -1263,7 +1263,7 @@ describe("Account view", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const getAccountBalancesSpy = jest diff --git a/extension/src/popup/views/__tests__/SignTransaction.test.tsx b/extension/src/popup/views/__tests__/SignTransaction.test.tsx index 9f4b149611..0023226432 100644 --- a/extension/src/popup/views/__tests__/SignTransaction.test.tsx +++ b/extension/src/popup/views/__tests__/SignTransaction.test.tsx @@ -346,7 +346,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); @@ -463,7 +463,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -603,7 +603,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); @@ -718,7 +718,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -841,7 +841,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -964,7 +964,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -1081,7 +1081,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -1204,7 +1204,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); @@ -1344,7 +1344,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -1519,7 +1519,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( @@ -1686,7 +1686,7 @@ describe("SignTransactions", () => { isNonSSLEnabled: false, experimentalFeaturesState: SettingsState.SUCCESS, assetsLists: DEFAULT_ASSETS_LISTS, - autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, }), ); const transaction = TransactionBuilder.fromXDR( From e710fa1c393bb7adc2aaec0597a841e007b62be6 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 21 May 2026 17:21:18 -0700 Subject: [PATCH 03/21] Address PR review feedback: treat shortened-timeout save as activity; simplify userActivity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewer questions on PR #2802: 1. saveSettings.ts: when the user shortens the auto-lock timeout, the save itself is a user action — we should rearm the timer with the new timeout, not synthesize an immediate lock just because the elapsed-idle time of the in-flight alarm exceeds the new threshold. Remove the elapsed-idle / immediate-lock branch entirely along with the previousAutoLockTimeoutMinutes capture, the existingAlarm inspection, and the wasLocked round-trip. 2. userActivity.ts: a USER_ACTIVITY ping cannot arrive on a locked wallet by design — the popup-side useActivityPing hook only attaches event listeners while unlocked, and popupMessageListener gates the message behind isFromExtensionPage. Drop the redundant in-handler unlocked check (and its dependencies on sessionStore / buildHasPrivateKeySelector / localStore); the handler now just delegates to sessionTimer.resetSession. Knock-on cleanups: - Remove the wasLocked field from saveSettings's return type, from the @shared/api/internal saveSettings response, and from the popup settings thunk; drop the lockAccount dispatch in that thunk. - Remove the now-unused lockAccount reducer / action from popup/ducks/accountServices. - Update tests: loadSaveSettings.test.ts now asserts that shortening the timeout rearms (rather than locking); userActivity.test.ts collapses to a single "always rearms" case; settings.test.ts verifies the popup auth slice is left untouched after save. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/api/internal.ts | 3 +- .../__tests__/loadSaveSettings.test.ts | 46 +++--------- .../__tests__/userActivity.test.ts | 73 ++----------------- .../messageListener/handlers/saveSettings.ts | 48 ++---------- .../messageListener/handlers/userActivity.ts | 42 +++-------- .../messageListener/popupMessageListener.ts | 2 +- .../popup/ducks/__tests__/settings.test.ts | 28 ++----- extension/src/popup/ducks/accountServices.ts | 4 - extension/src/popup/ducks/settings.ts | 18 +---- 9 files changed, 50 insertions(+), 214 deletions(-) diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 8597d7101b..a79a8a902e 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -1630,7 +1630,7 @@ export const saveSettings = async ({ isHideDustEnabled: boolean; isOpenSidebarByDefault: boolean; autoLockTimeoutMinutes: AutoLockTimeoutMinutes; -}): Promise => { +}): Promise => { let response = { allowList: DEFAULT_ALLOW_LIST, isDataSharingAllowed: false, @@ -1647,7 +1647,6 @@ export const saveSettings = async ({ autoLockTimeoutMinutes: DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, error: "", hiddenAssets: {}, - wasLocked: false, }; try { diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts index 6ac0ad1320..b0ddfe697b 100644 --- a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -226,7 +226,6 @@ describe("saveSettings autoLockTimeoutMinutes", () => { }); it("persists a valid timeout and reschedules when unlocked", async () => { - alarmsGet.mockResolvedValue(undefined); const localStore = makeLocalStore(30); const sessionTimer = makeSessionTimer(); @@ -243,11 +242,10 @@ describe("saveSettings autoLockTimeoutMinutes", () => { ); expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); expect((result as any).autoLockTimeoutMinutes).toBe(30); - expect((result as any).wasLocked).toBe(false); + expect((result as any).wasLocked).toBeUndefined(); }); it("does not reschedule the timer when the wallet is locked", async () => { - alarmsGet.mockResolvedValue(undefined); const sessionTimer = makeSessionTimer(); await saveSettings({ request: { ...baseRequest, autoLockTimeoutMinutes: 5 } as any, @@ -256,15 +254,15 @@ describe("saveSettings autoLockTimeoutMinutes", () => { sessionTimer, }); expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + expect(sessionTimer.stopSession).not.toHaveBeenCalled(); }); - it("locks immediately when the new timeout has already elapsed", async () => { - // Previously 60 min, alarm was scheduled to fire in 10 min → 50 min - // already elapsed. User shrinks the timeout to 30 min — that - // threshold has already passed, so we lock now rather than rearm. - alarmsGet.mockResolvedValue({ - scheduledTime: Date.now() + 10 * 60_000, - }); + it("rearms (rather than locking) when the user shortens the timeout", async () => { + // Saving settings is itself a user action, so shortening the + // timeout should restart the idle clock with the new value rather + // than synthesizing an immediate lock — even when the new threshold + // is already smaller than the elapsed idle time of the in-flight + // alarm. const localStore = makeLocalStore(60); const sessionTimer = makeSessionTimer(); const sessionStore = { @@ -273,38 +271,16 @@ describe("saveSettings autoLockTimeoutMinutes", () => { } as any; const result = await saveSettings({ - request: { ...baseRequest, autoLockTimeoutMinutes: 30 } as any, + request: { ...baseRequest, autoLockTimeoutMinutes: 5 } as any, localStore, sessionStore, sessionTimer, }); - expect(sessionTimer.stopSession).toHaveBeenCalledTimes(1); - expect(sessionTimer.resetSession).not.toHaveBeenCalled(); - expect(sessionStore.dispatch).toHaveBeenCalled(); - expect((result as any).wasLocked).toBe(true); - }); - - it("rearms when the new timeout still has time remaining", async () => { - // Previously 60 min, alarm scheduled to fire in 50 min → 10 min - // already elapsed. User shrinks to 30 min — still within budget, - // so we just rearm at +30. - alarmsGet.mockResolvedValue({ - scheduledTime: Date.now() + 50 * 60_000, - }); - const localStore = makeLocalStore(60); - const sessionTimer = makeSessionTimer(); - - const result = await saveSettings({ - request: { ...baseRequest, autoLockTimeoutMinutes: 30 } as any, - localStore, - sessionStore: makeSessionStore("k"), - sessionTimer, - }); - expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); expect(sessionTimer.stopSession).not.toHaveBeenCalled(); - expect((result as any).wasLocked).toBe(false); + expect(sessionStore.dispatch).not.toHaveBeenCalled(); + expect((result as any).wasLocked).toBeUndefined(); }); }); diff --git a/extension/src/background/messageListener/__tests__/userActivity.test.ts b/extension/src/background/messageListener/__tests__/userActivity.test.ts index 2dcadf2e34..1e40e83cdf 100644 --- a/extension/src/background/messageListener/__tests__/userActivity.test.ts +++ b/extension/src/background/messageListener/__tests__/userActivity.test.ts @@ -1,14 +1,5 @@ import { userActivity } from "../handlers/userActivity"; -jest.mock("background/helpers/account", () => ({ - getIsHardwareWalletActive: jest.fn().mockResolvedValue(false), -})); - -const makeSessionStore = (state: any) => - ({ - getState: () => ({ session: state }), - }) as any; - const makeSessionTimer = () => ({ resetSession: jest.fn().mockResolvedValue(undefined), @@ -16,65 +7,15 @@ const makeSessionTimer = () => stopSession: jest.fn().mockResolvedValue(undefined), }) as any; -const makeLocalStore = () => - ({ - getItem: jest.fn().mockResolvedValue(null), - setItem: jest.fn(), - remove: jest.fn(), - }) as any; - describe("userActivity handler", () => { - it("resets the session timer when the wallet is unlocked (hot wallet)", async () => { - const sessionTimer = makeSessionTimer(); - const result = await userActivity({ - sessionStore: makeSessionStore({ hashKey: { key: "deadbeef" } }), - sessionTimer, - localStore: makeLocalStore(), - }); - expect(result).toEqual({ ok: true }); - expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); - }); - - it("rejects when the wallet is locked", async () => { - const sessionTimer = makeSessionTimer(); - const result = await userActivity({ - sessionStore: makeSessionStore({ hashKey: { key: "" } }), - sessionTimer, - localStore: makeLocalStore(), - }); - expect(result).toEqual({ ok: false }); - expect(sessionTimer.resetSession).not.toHaveBeenCalled(); - }); - - it("rejects when a hardware wallet is active but locked", async () => { - const { getIsHardwareWalletActive } = jest.requireMock( - "background/helpers/account", - ); - (getIsHardwareWalletActive as jest.Mock).mockResolvedValueOnce(true); - const sessionTimer = makeSessionTimer(); - const result = await userActivity({ - sessionStore: makeSessionStore({ - hashKey: { key: "" }, - isHardwareWalletLocked: true, - }), - sessionTimer, - localStore: makeLocalStore(), - }); - expect(result).toEqual({ ok: false }); - expect(sessionTimer.resetSession).not.toHaveBeenCalled(); - }); - - it("resets when a hardware wallet is active and unlocked", async () => { - const { getIsHardwareWalletActive } = jest.requireMock( - "background/helpers/account", - ); - (getIsHardwareWalletActive as jest.Mock).mockResolvedValueOnce(true); + it("resets the session timer on every ping", async () => { + // The caller-side guards (popup-side `useActivityPing` only attaches + // listeners while unlocked, and `popupMessageListener` gates this + // message behind `isFromExtensionPage`) mean the handler is reached + // only on genuine user activity. The handler itself unconditionally + // rearms the idle alarm. const sessionTimer = makeSessionTimer(); - const result = await userActivity({ - sessionStore: makeSessionStore({ hashKey: { key: "" } }), - sessionTimer, - localStore: makeLocalStore(), - }); + const result = await userActivity({ sessionTimer }); expect(result).toEqual({ ok: true }); expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); }); diff --git a/extension/src/background/messageListener/handlers/saveSettings.ts b/extension/src/background/messageListener/handlers/saveSettings.ts index aacf5d1e9d..5638b3c59b 100644 --- a/extension/src/background/messageListener/handlers/saveSettings.ts +++ b/extension/src/background/messageListener/handlers/saveSettings.ts @@ -1,4 +1,3 @@ -import browser from "webextension-polyfill"; import { Store } from "redux"; import { SaveSettingsMessage } from "@shared/api/types/message-request"; @@ -20,11 +19,7 @@ import { buildHasPrivateKeySelector, SessionState, } from "background/ducks/session"; -import { - clearSession, - SESSION_ALARM_NAME, - SessionTimer, -} from "background/helpers/session"; +import { SessionTimer } from "background/helpers/session"; import { AUTO_LOCK_TIMEOUT_MINUTES_ID, DATA_SHARING_ID, @@ -56,12 +51,6 @@ export const saveSettings = async ({ return { error: "Invalid autoLockTimeoutMinutes" }; } - // Capture the previous timeout *before* writing the new one so we can - // reason about elapsed-idle time against the alarm currently in flight. - const previousAutoLockTimeoutMinutes = coerceAutoLockTimeoutMinutes( - await localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID), - ); - await localStore.setItem(DATA_SHARING_ID, isDataSharingAllowed); await localStore.setItem(IS_VALIDATING_MEMO_ID, isMemoValidationEnabled); await localStore.setItem(IS_HIDE_DUST_ENABLED_ID, isHideDustEnabled); @@ -74,38 +63,18 @@ export const saveSettings = async ({ autoLockTimeoutMinutes, ); - // A new auto-lock timeout takes effect immediately, but only if the - // wallet is currently unlocked. When shortening the timeout, the user - // may already have been idle longer than the new threshold — in that - // case we lock immediately rather than schedule an alarm in the past. - // `wasLocked` is propagated to the popup so its `auth.hasPrivateKey` - // can flip without waiting for the next `useGetAppData` poll. - let wasLocked = false; + // Saving settings is itself a user action, so it counts as activity: + // rearm the idle timer with the new timeout rather than synthesizing + // an immediate lock when the new threshold is shorter than the elapsed + // idle time. Only rearm if the wallet is currently unlocked — for a + // locked wallet there is no session to protect and we'd just leave a + // stray alarm pending. const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); const isUnlocked = await hasPrivateKeySelector( sessionStore.getState() as SessionState, ); if (isUnlocked) { - const existingAlarm = await browser.alarms.get(SESSION_ALARM_NAME); - if (!existingAlarm) { - // Recovery edge: the worker just woke and the alarm hasn't been - // re-observed yet (or this is the first save after unlock). - // Don't synthesize an immediate lock; just rearm with the new - // timeout. - await sessionTimer.resetSession(); - } else { - const newDelayMs = autoLockTimeoutMinutes * 60_000; - const oldDelayMs = previousAutoLockTimeoutMinutes * 60_000; - const remainingMs = existingAlarm.scheduledTime - Date.now(); - const elapsedIdleMs = Math.max(0, oldDelayMs - remainingMs); - if (elapsedIdleMs >= newDelayMs) { - await clearSession({ sessionStore, localStore }); - await sessionTimer.stopSession(); - wasLocked = true; - } else { - await sessionTimer.resetSession(); - } - } + await sessionTimer.resetSession(); } // Apply sidebar behavior immediately on Chrome @@ -135,6 +104,5 @@ export const saveSettings = async ({ autoLockTimeoutMinutes: coerceAutoLockTimeoutMinutes( await localStore.getItem(AUTO_LOCK_TIMEOUT_MINUTES_ID), ), - wasLocked, }; }; diff --git a/extension/src/background/messageListener/handlers/userActivity.ts b/extension/src/background/messageListener/handlers/userActivity.ts index 1c6080e57b..006a579907 100644 --- a/extension/src/background/messageListener/handlers/userActivity.ts +++ b/extension/src/background/messageListener/handlers/userActivity.ts @@ -1,46 +1,26 @@ -import { Store } from "redux"; - -import { - buildHasPrivateKeySelector, - SessionState, -} from "background/ducks/session"; import { SessionTimer } from "background/helpers/session"; -import { DataStorageAccess } from "background/helpers/dataStorageAccess"; /** * Handle a USER_ACTIVITY ping from an extension page. * - * When the wallet is unlocked, this rearms the idle auto-lock alarm - * via `sessionTimer.resetSession()`. "Unlocked" means either the - * hot-wallet `hashKey` is present OR a hardware wallet is active and - * not idle-locked — `buildHasPrivateKeySelector` is the canonical - * predicate, used by the popup router for the same purpose. - * - * When locked we reject the ping — a stale activity listener in a - * still-mounted extension page must not re-arm the alarm after the - * user has signed out or auto-locked elsewhere. + * Rearms the idle auto-lock alarm by delegating to + * `sessionTimer.resetSession()`. The popup-side `useActivityPing` hook + * only attaches event listeners while the wallet is unlocked, and + * `popupMessageListener` gates this message behind `isFromExtensionPage` + * so dApp content scripts cannot reach it — so a ping arriving here is + * already proof of genuine user activity in an unlocked extension page. * - * Caller-side, `popupMessageListener` additionally gates this message - * behind `isFromExtensionPage` so dApp content scripts cannot extend - * an unlocked session. + * In the unlikely race where the wallet locks between the user's input + * and the popup tearing down its listeners, an extra `resetSession()` + * just (re)schedules an alarm that will fire on a locked session and + * be a no-op when `clearSession` runs against state that's already + * cleared. */ export const userActivity = async ({ - sessionStore, sessionTimer, - localStore, }: { - sessionStore: Store; sessionTimer: SessionTimer; - localStore: DataStorageAccess; }) => { - const hasPrivateKeySelector = buildHasPrivateKeySelector(localStore); - const isUnlocked = await hasPrivateKeySelector( - sessionStore.getState() as SessionState, - ); - if (!isUnlocked) { - return { ok: false }; - } - await sessionTimer.resetSession(); return { ok: true }; }; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 49d1441c5e..e549321b31 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -631,7 +631,7 @@ export const popupMessageListener = ( case SERVICE_TYPES.USER_ACTIVITY: { if (!isFromExtensionPage) return { error: "Unauthorized" }; - return userActivity({ sessionStore, sessionTimer, localStore }); + return userActivity({ sessionTimer }); } default: diff --git a/extension/src/popup/ducks/__tests__/settings.test.ts b/extension/src/popup/ducks/__tests__/settings.test.ts index b4862957ba..7cd929c163 100644 --- a/extension/src/popup/ducks/__tests__/settings.test.ts +++ b/extension/src/popup/ducks/__tests__/settings.test.ts @@ -11,7 +11,7 @@ jest.mock("@shared/api/internal", () => ({ saveSettings: jest.fn(), })); -const makeSettingsResponse = (wasLocked?: boolean) => ({ +const makeSettingsResponse = () => ({ allowList: {}, isDataSharingAllowed: true, isMemoValidationEnabled: true, @@ -29,7 +29,6 @@ const makeSettingsResponse = (wasLocked?: boolean) => ({ isSorobanPublicEnabled: false, settingsState: SettingsState.SUCCESS, userNotification: { enabled: false, message: "" }, - wasLocked, }); const makeStore = () => @@ -65,28 +64,15 @@ describe("settings saveSettings thunk", () => { (saveSettingsService as jest.Mock).mockReset(); }); - it("dispatches lockAccount when the background reports an immediate lock", async () => { - (saveSettingsService as jest.Mock).mockResolvedValue( - makeSettingsResponse(true), - ); + it("leaves the popup auth state untouched after a successful save", async () => { + // Background no longer reports an immediate-lock back to the popup — + // saving settings is treated as user activity and the timer is + // simply rearmed. The auth slice should remain unlocked. + (saveSettingsService as jest.Mock).mockResolvedValue(makeSettingsResponse()); const store = makeStore(); await store.dispatch(saveSettings(request) as any); - expect(store.getState().auth.hasPrivateKey).toBe(false); + expect(store.getState().auth.hasPrivateKey).toBe(true); }); - - it.each([false, undefined])( - "does not lock the popup auth state when wasLocked is %s", - async (wasLocked) => { - (saveSettingsService as jest.Mock).mockResolvedValue( - makeSettingsResponse(wasLocked), - ); - const store = makeStore(); - - await store.dispatch(saveSettings(request) as any); - - expect(store.getState().auth.hasPrivateKey).toBe(true); - }, - ); }); diff --git a/extension/src/popup/ducks/accountServices.ts b/extension/src/popup/ducks/accountServices.ts index 920e94bfea..a397ebafd6 100644 --- a/extension/src/popup/ducks/accountServices.ts +++ b/extension/src/popup/ducks/accountServices.ts @@ -570,9 +570,6 @@ const authSlice = createSlice({ clearApiError(state) { state.error = ""; }, - lockAccount(state) { - state.hasPrivateKey = false; - }, setConnectingWalletType(state, action) { state.connectingWalletType = action.payload; }, @@ -993,7 +990,6 @@ export const { saveAccount, saveAccountError, saveApplicationState, - lockAccount, } = authSlice.actions; export { reducer }; diff --git a/extension/src/popup/ducks/settings.ts b/extension/src/popup/ducks/settings.ts index 9f8fabfe92..9a3a4c1bf0 100644 --- a/extension/src/popup/ducks/settings.ts +++ b/extension/src/popup/ducks/settings.ts @@ -40,7 +40,7 @@ import { SettingsState, ExperimentalFeatures, } from "@shared/api/types"; -import { lockAccount, publicKeySelector } from "popup/ducks/accountServices"; +import { publicKeySelector } from "popup/ducks/accountServices"; import { AppState } from "popup/App"; import { isMainnet } from "helpers/stellar"; @@ -126,7 +126,7 @@ export const saveAllowList = createAsyncThunk< ); export const saveSettings = createAsyncThunk< - Settings & IndexerSettings & { wasLocked?: boolean }, + Settings & IndexerSettings, { isDataSharingAllowed: boolean; isMemoValidationEnabled: boolean; @@ -145,9 +145,9 @@ export const saveSettings = createAsyncThunk< isOpenSidebarByDefault, autoLockTimeoutMinutes, }, - { dispatch, getState, rejectWithValue }, + { getState, rejectWithValue }, ) => { - let res: Settings & IndexerSettings & { wasLocked?: boolean } = { + let res: Settings & IndexerSettings = { ...settingsInitialState, isSorobanPublicEnabled: false, isRpcHealthy: false, @@ -155,7 +155,6 @@ export const saveSettings = createAsyncThunk< settingsState: SettingsState.IDLE, isHideDustEnabled: true, isOpenSidebarByDefault: false, - wasLocked: false, }; const activePublicKey = publicKeySelector(getState()); @@ -176,15 +175,6 @@ export const saveSettings = createAsyncThunk< }); } - // When the background locks the session because the shortened - // timeout has already elapsed, flip the popup's auth slice - // immediately rather than waiting for the next `useGetAppData` - // poll — otherwise the user briefly sees an unlocked UI after - // saving. - if (res.wasLocked) { - dispatch(lockAccount()); - } - return res; }, ); From 0c1d6cadef06bc0c8ecc6e4d821a4db4f2e92bbe Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 21 May 2026 17:32:21 -0700 Subject: [PATCH 04/21] Strengthen shortened-timeout regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal review-fix flagged the new "rearms when timeout is shortened" test as too weak — without an alarmsGet stub the parent implementation resetSession(), so the test could not distinguish parent from current behavior. Mock alarmsGet to return an in-flight alarm whose elapsed idle time (59 min) far exceeds the new 5 min timeout: that is the exact scenario where the parent code would have synthesized an immediate lock. The current implementation must just rearm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../messageListener/__tests__/loadSaveSettings.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts index b0ddfe697b..3d63adb689 100644 --- a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -262,7 +262,14 @@ describe("saveSettings autoLockTimeoutMinutes", () => { // timeout should restart the idle clock with the new value rather // than synthesizing an immediate lock — even when the new threshold // is already smaller than the elapsed idle time of the in-flight - // alarm. + // alarm. Set up `alarmsGet` to return an alarm whose remaining time + // (1 min) is much less than the new 5 min timeout, i.e. elapsed + // idle (59 min) ≫ new timeout (5 min). The previous implementation + // would have detected this and locked immediately; the current + // implementation must simply rearm. + alarmsGet.mockResolvedValue({ + scheduledTime: Date.now() + 1 * 60_000, + }); const localStore = makeLocalStore(60); const sessionTimer = makeSessionTimer(); const sessionStore = { From b00599bc8c3b164d2c02e1fa774d0bdfb9f3a27f Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 09:59:15 -0700 Subject: [PATCH 05/21] Flip popup to unlock screen when idle auto-lock fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous PR locked the wallet on the background side (cleared the hashKey via timeoutAccountAccess + lockHardwareWallet) but did not notify any open extension UI surface. The popup, sidebar, and standalone signing/grant-access windows kept rendering whichever view they were on — Account, Assets, History — and the user only learned the wallet had locked when they tried to take a private-key-requiring action and were ambushed by a password prompt mid-flow. Make the background broadcast a SESSION_LOCKED runtime message after clearSession runs in the alarm handler. Add a SessionLockListener mounted inside the Router (so it has access to useNavigate) that dispatches a new lockAccount reducer (clearing hasPrivateKey, publicKey, allAccounts, bipPath, tokenIdList) and navigates to ROUTES.unlockAccount. applicationState is left untouched so still renders rather than redirecting to . The mechanism mirrors the original PR description's intent — a push-based notification from background to popup so the UI flip is not gated on the next useGetAppData refetch — but applies to the regular idle-alarm path that was missed when the shortened-timeout immediate-lock variant was simplified out earlier in review. Tests added: - background/__tests__/initAlarmListener.test.ts — broadcast happens after clearSession; sendMessage rejection (no receivers) is swallowed; unrelated alarms are ignored. - popup/ducks/__tests__/accountServices.test.ts — lockAccount clears private-key-derived state and preserves applicationState. - popup/components/__tests__/SessionLockListener.test.tsx — listener registers/unregisters, dispatches lockAccount + navigates on SESSION_LOCKED, ignores unrelated messages. Addresses https://github.com/stellar/freighter/pull/2802#pullrequestreview-3439015700 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/constants/services.ts | 1 + .../__tests__/initAlarmListener.test.ts | 86 +++++++++++++ extension/src/background/index.ts | 14 +++ extension/src/popup/Router.tsx | 2 + .../components/SessionLockListener/index.tsx | 42 +++++++ .../__tests__/SessionLockListener.test.tsx | 117 ++++++++++++++++++ .../ducks/__tests__/accountServices.test.ts | 46 +++++++ extension/src/popup/ducks/accountServices.ts | 18 +++ 8 files changed, 326 insertions(+) create mode 100644 extension/src/background/__tests__/initAlarmListener.test.ts create mode 100644 extension/src/popup/components/SessionLockListener/index.tsx create mode 100644 extension/src/popup/components/__tests__/SessionLockListener.test.tsx create mode 100644 extension/src/popup/ducks/__tests__/accountServices.test.ts diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index f7d9297bb4..7f65e3c984 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -71,6 +71,7 @@ export enum SERVICE_TYPES { GET_DISCOVER_WELCOME_SEEN = "GET_DISCOVER_WELCOME_SEEN", DISMISS_DISCOVER_WELCOME = "DISMISS_DISCOVER_WELCOME", USER_ACTIVITY = "USER_ACTIVITY", + SESSION_LOCKED = "SESSION_LOCKED", } // SIDEBAR_NAVIGATE is a plain string constant (not in an enum) because it is diff --git a/extension/src/background/__tests__/initAlarmListener.test.ts b/extension/src/background/__tests__/initAlarmListener.test.ts new file mode 100644 index 0000000000..fc11d779c6 --- /dev/null +++ b/extension/src/background/__tests__/initAlarmListener.test.ts @@ -0,0 +1,86 @@ +import browser from "webextension-polyfill"; + +import { SERVICE_TYPES } from "@shared/constants/services"; + +let alarmHandler: + | ((alarm: { name: string }) => void | Promise) + | undefined; + +const mockSendMessage = jest.fn().mockResolvedValue(undefined); + +jest.mock("webextension-polyfill", () => ({ + alarms: { + onAlarm: { + addListener: jest.fn((handler) => { + alarmHandler = handler; + }), + }, + }, + runtime: { + sendMessage: (...args: any[]) => mockSendMessage(...args), + }, +})); + +const mockClearSession = jest.fn().mockResolvedValue(undefined); +jest.mock("background/helpers/session", () => { + const actual = jest.requireActual("background/helpers/session"); + return { + ...actual, + clearSession: (...args: any[]) => mockClearSession(...args), + }; +}); + +jest.mock("background/store", () => ({ + buildStore: jest.fn().mockResolvedValue({}), +})); + +jest.mock("background/helpers/dataStorageAccess", () => ({ + dataStorageAccess: jest.fn().mockReturnValue({}), + browserLocalStorage: {}, +})); + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { initAlarmListener } = require("background/index"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { SESSION_ALARM_NAME } = require("background/helpers/session"); + +describe("initAlarmListener", () => { + beforeEach(() => { + mockSendMessage.mockClear(); + mockClearSession.mockClear(); + alarmHandler = undefined; + initAlarmListener(); + }); + + it("clears the session and broadcasts SESSION_LOCKED when the auto-lock alarm fires", async () => { + expect(browser.alarms.onAlarm.addListener).toHaveBeenCalled(); + expect(alarmHandler).toBeDefined(); + + await alarmHandler!({ name: SESSION_ALARM_NAME }); + + expect(mockClearSession).toHaveBeenCalledTimes(1); + expect(mockSendMessage).toHaveBeenCalledTimes(1); + expect(mockSendMessage).toHaveBeenCalledWith({ + type: SERVICE_TYPES.SESSION_LOCKED, + }); + }); + + it("swallows sendMessage errors when no UI is open to receive the broadcast", async () => { + mockSendMessage.mockRejectedValueOnce( + new Error("Could not establish connection. Receiving end does not exist."), + ); + + await expect( + alarmHandler!({ name: SESSION_ALARM_NAME }), + ).resolves.toBeUndefined(); + + expect(mockClearSession).toHaveBeenCalledTimes(1); + }); + + it("ignores unrelated alarms", async () => { + await alarmHandler!({ name: "some-other-alarm" }); + + expect(mockClearSession).not.toHaveBeenCalled(); + expect(mockSendMessage).not.toHaveBeenCalled(); + }); +}); diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index fddcd38f18..d21f4baa56 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -231,6 +231,20 @@ export const initAlarmListener = () => { if (name === SESSION_ALARM_NAME) { await clearSession({ sessionStore, localStore }); + // Broadcast to every open extension UI (popup, sidebar, standalone + // signing/grant-access windows) that the wallet just auto-locked, so + // they can flip to the unlock screen instead of leaving stale, + // privilege-suggestive views (Account, Assets, History) on screen + // until the next data refetch. `runtime.sendMessage` only reaches + // extension contexts other than the sender; when no UI is open it + // rejects with "Could not establish connection", which is harmless. + try { + await browser.runtime.sendMessage({ + type: SERVICE_TYPES.SESSION_LOCKED, + }); + } catch { + // No receivers — nothing to do. + } } }); }; diff --git a/extension/src/popup/Router.tsx b/extension/src/popup/Router.tsx index 457e8e5257..5684a55d88 100644 --- a/extension/src/popup/Router.tsx +++ b/extension/src/popup/Router.tsx @@ -66,6 +66,7 @@ import { ConfirmSidebarRequest } from "popup/views/ConfirmSidebarRequest"; import { DEV_SERVER } from "@shared/constants/services"; import { isSidebarMode } from "popup/helpers/isSidebarMode"; import { SidebarSigningListener } from "popup/components/SidebarSigningListener"; +import { SessionLockListener } from "popup/components/SessionLockListener"; import { SettingsState } from "@shared/api/types"; import { SignMessage } from "./views/SignMessage"; @@ -173,6 +174,7 @@ const Layout = () => { export const Router = () => ( + {isSidebarMode() && } }> diff --git a/extension/src/popup/components/SessionLockListener/index.tsx b/extension/src/popup/components/SessionLockListener/index.tsx new file mode 100644 index 0000000000..45b7885855 --- /dev/null +++ b/extension/src/popup/components/SessionLockListener/index.tsx @@ -0,0 +1,42 @@ +import { useEffect } from "react"; +import { useDispatch } from "react-redux"; +import { useNavigate } from "react-router-dom"; +import browser from "webextension-polyfill"; + +import { SERVICE_TYPES } from "@shared/constants/services"; +import { ROUTES } from "popup/constants/routes"; +import { lockAccount } from "popup/ducks/accountServices"; + +/** + * Listens for the background's `SESSION_LOCKED` broadcast (fired when + * the idle auto-lock alarm elapses) and flips the popup over to the + * unlock screen immediately, instead of leaving stale account/asset + * views on screen until the next data refetch. + * + * Mounted inside `` so it can use `useNavigate`. Every + * Freighter UI surface (popup, sidebar, standalone signing window, + * grant-access window) renders the same `` → `` tree, so + * one mount covers all of them. + */ +export const SessionLockListener = () => { + const dispatch = useDispatch(); + const navigate = useNavigate(); + + useEffect(() => { + const handler = (message: unknown) => { + if (typeof message !== "object" || message === null) return undefined; + const { type } = message as { type?: unknown }; + if (type !== SERVICE_TYPES.SESSION_LOCKED) return undefined; + dispatch(lockAccount()); + navigate(ROUTES.unlockAccount); + return undefined; + }; + + browser.runtime.onMessage.addListener(handler); + return () => { + browser.runtime.onMessage.removeListener(handler); + }; + }, [dispatch, navigate]); + + return null; +}; diff --git a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx new file mode 100644 index 0000000000..fa7ec138ba --- /dev/null +++ b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx @@ -0,0 +1,117 @@ +import React from "react"; +import { render, act } from "@testing-library/react"; +import { Provider } from "react-redux"; +import { combineReducers, configureStore } from "@reduxjs/toolkit"; +import { MemoryRouter, Routes, Route, useLocation } from "react-router-dom"; +import browser from "webextension-polyfill"; + +import { SERVICE_TYPES } from "@shared/constants/services"; +import { ROUTES } from "popup/constants/routes"; +import { reducer as authReducer } from "popup/ducks/accountServices"; +import { SessionLockListener } from "../SessionLockListener"; + +type RuntimeHandler = (message: unknown) => void; + +const listeners: RuntimeHandler[] = []; + +jest.mock("webextension-polyfill", () => ({ + runtime: { + onMessage: { + addListener: jest.fn((h: RuntimeHandler) => listeners.push(h)), + removeListener: jest.fn((h: RuntimeHandler) => { + const idx = listeners.indexOf(h); + if (idx >= 0) listeners.splice(idx, 1); + }), + }, + }, +})); + +const makeStore = () => + configureStore({ + reducer: combineReducers({ auth: authReducer }), + preloadedState: { + auth: { + allAccounts: [{ publicKey: "GBTEST" } as any], + migratedAccounts: [], + applicationState: "MNEMONIC_PHRASE_CONFIRMED", + hasPrivateKey: true, + publicKey: "GBTEST", + connectingWalletType: "NONE", + bipPath: "m/0", + tokenIdList: [], + error: "", + accountStatus: "IDLE", + isAccountMismatch: false, + }, + } as any, + }); + +let currentPath = "/"; +const PathSpy = () => { + const location = useLocation(); + currentPath = location.pathname; + return null; +}; + +const renderListener = (store = makeStore()) => + render( + + + + + + + + + , + ); + +describe("SessionLockListener", () => { + beforeEach(() => { + listeners.length = 0; + currentPath = "/"; + jest.clearAllMocks(); + }); + + it("registers a runtime.onMessage listener on mount and removes it on unmount", () => { + const { unmount } = renderListener(); + + expect(browser.runtime.onMessage.addListener).toHaveBeenCalledTimes(1); + expect(listeners).toHaveLength(1); + + unmount(); + expect(browser.runtime.onMessage.removeListener).toHaveBeenCalledTimes(1); + expect(listeners).toHaveLength(0); + }); + + it("dispatches lockAccount and navigates to unlockAccount on SESSION_LOCKED", () => { + const store = makeStore(); + renderListener(store); + + act(() => { + listeners[0]({ type: SERVICE_TYPES.SESSION_LOCKED }); + }); + + const { auth } = store.getState(); + expect(auth.hasPrivateKey).toBe(false); + expect(auth.publicKey).toBe(""); + expect(auth.allAccounts).toEqual([]); + expect(currentPath).toBe(ROUTES.unlockAccount); + }); + + it("ignores unrelated messages", () => { + const store = makeStore(); + renderListener(store); + + act(() => { + listeners[0]({ type: SERVICE_TYPES.LOAD_ACCOUNT }); + listeners[0]("not an object"); + listeners[0](null); + }); + + const { auth } = store.getState(); + expect(auth.hasPrivateKey).toBe(true); + expect(auth.publicKey).toBe("GBTEST"); + expect(currentPath).toBe("/"); + }); +}); diff --git a/extension/src/popup/ducks/__tests__/accountServices.test.ts b/extension/src/popup/ducks/__tests__/accountServices.test.ts new file mode 100644 index 0000000000..1ff9aa5713 --- /dev/null +++ b/extension/src/popup/ducks/__tests__/accountServices.test.ts @@ -0,0 +1,46 @@ +import { combineReducers, configureStore } from "@reduxjs/toolkit"; + +import { + reducer as authReducer, + lockAccount, +} from "../accountServices"; + +const makeStore = () => + configureStore({ + reducer: combineReducers({ auth: authReducer }), + preloadedState: { + auth: { + allAccounts: [ + { publicKey: "GBTEST", name: "Account 1", imported: false } as any, + ], + migratedAccounts: [], + applicationState: "MNEMONIC_PHRASE_CONFIRMED", + hasPrivateKey: true, + publicKey: "GBTEST", + connectingWalletType: "NONE", + bipPath: "m/44'/148'/0'", + tokenIdList: ["token1"], + error: "", + accountStatus: "IDLE", + isAccountMismatch: false, + }, + } as any, + }); + +describe("accountServices lockAccount reducer", () => { + it("clears private-key-derived state but preserves applicationState", () => { + const store = makeStore(); + + store.dispatch(lockAccount()); + + const { auth } = store.getState(); + expect(auth.hasPrivateKey).toBe(false); + expect(auth.publicKey).toBe(""); + expect(auth.allAccounts).toEqual([]); + expect(auth.bipPath).toBe(""); + expect(auth.tokenIdList).toEqual([]); + // Left intact so `` still renders `` + // instead of redirecting to ``. + expect(auth.applicationState).toBe("MNEMONIC_PHRASE_CONFIRMED"); + }); +}); diff --git a/extension/src/popup/ducks/accountServices.ts b/extension/src/popup/ducks/accountServices.ts index a397ebafd6..336a8e8bcc 100644 --- a/extension/src/popup/ducks/accountServices.ts +++ b/extension/src/popup/ducks/accountServices.ts @@ -603,6 +603,23 @@ const authSlice = createSlice({ saveApplicationState(state, action) { state.applicationState = action.payload; }, + /** + * Called when the background broadcasts that the wallet has been + * locked (e.g., the idle auto-lock alarm fired). Mirrors the + * background's `timeoutAccountAccess` reset on the popup side so + * that route guards (`VerifiedAccountRoute`, `useGetAppData`) see + * `hasPrivateKey: false` immediately instead of after the next + * data refetch. Public/private-key-derived state is cleared; + * `applicationState` is left untouched so `` + * still renders `` rather than ``. + */ + lockAccount(state) { + state.hasPrivateKey = false; + state.publicKey = ""; + state.allAccounts = []; + state.bipPath = ""; + state.tokenIdList = []; + }, }, extraReducers: (builder) => { builder.addCase(createAccount.fulfilled, (state, action) => { @@ -990,6 +1007,7 @@ export const { saveAccount, saveAccountError, saveApplicationState, + lockAccount, } = authSlice.actions; export { reducer }; From ecf489b60a1a49cb64ac7cf8d9f5412800064ed5 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 10:18:24 -0700 Subject: [PATCH 06/21] Preserve interrupted route context across auto-lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal review-fix round identified that the previous commit dropped location.state.from and location.search when navigating to the unlock screen on SESSION_LOCKED. UnlockAccount reads state.from.pathname + location.search to return the user to the interrupted flow after a successful unlock — without this, a user auto-locked mid-grant-access or mid-signing was returned to the default Account page on unlock rather than back into the approval flow, exactly the way SignTransaction and GrantAccess already preserve context for their own reroutes. Capture useLocation() in SessionLockListener and navigate with {state: {from: location}} + location.search appended, mirroring the pattern used by the in-flow reroutes. Also short-circuit when the current pathname is already /unlock-account so a redundant broadcast cannot clobber a state.from already set by an earlier reroute (e.g. UnlockAccountRoute firing before the SESSION_LOCKED message arrives). Tests extended to cover: - state.from + location.search preserved from /grant-access?... - no-op when already on /unlock-account Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../components/SessionLockListener/index.tsx | 28 ++++-- .../__tests__/SessionLockListener.test.tsx | 85 ++++++++++++++----- 2 files changed, 86 insertions(+), 27 deletions(-) diff --git a/extension/src/popup/components/SessionLockListener/index.tsx b/extension/src/popup/components/SessionLockListener/index.tsx index 45b7885855..e2d9e297dd 100644 --- a/extension/src/popup/components/SessionLockListener/index.tsx +++ b/extension/src/popup/components/SessionLockListener/index.tsx @@ -1,6 +1,6 @@ import { useEffect } from "react"; import { useDispatch } from "react-redux"; -import { useNavigate } from "react-router-dom"; +import { useLocation, useNavigate } from "react-router-dom"; import browser from "webextension-polyfill"; import { SERVICE_TYPES } from "@shared/constants/services"; @@ -13,22 +13,36 @@ import { lockAccount } from "popup/ducks/accountServices"; * unlock screen immediately, instead of leaving stale account/asset * views on screen until the next data refetch. * - * Mounted inside `` so it can use `useNavigate`. Every - * Freighter UI surface (popup, sidebar, standalone signing window, - * grant-access window) renders the same `` → `` tree, so - * one mount covers all of them. + * Mounted inside `` so it can use `useNavigate` / + * `useLocation`. Every Freighter UI surface (popup, sidebar, standalone + * signing window, grant-access window) renders the same `` → + * `` tree, so one mount covers all of them. + * + * When navigating to the unlock screen we preserve the current + * `location` (as `state.from`) and `location.search`, mirroring the + * pattern used by `` / `` reroutes. After + * a successful unlock, `` reads `state.from.pathname` + + * `location.search` and returns the user to the interrupted flow + * (e.g. `/grant-access?...`, `/sign-transaction?...`) rather than + * stranding them on the default account page. */ export const SessionLockListener = () => { const dispatch = useDispatch(); const navigate = useNavigate(); + const location = useLocation(); useEffect(() => { const handler = (message: unknown) => { if (typeof message !== "object" || message === null) return undefined; const { type } = message as { type?: unknown }; if (type !== SERVICE_TYPES.SESSION_LOCKED) return undefined; + // Already on the unlock screen — nothing to do. Avoids clobbering + // an existing `state.from` set by an earlier reroute. + if (location.pathname === ROUTES.unlockAccount) return undefined; dispatch(lockAccount()); - navigate(ROUTES.unlockAccount); + navigate(`${ROUTES.unlockAccount}${location.search}`, { + state: { from: location }, + }); return undefined; }; @@ -36,7 +50,7 @@ export const SessionLockListener = () => { return () => { browser.runtime.onMessage.removeListener(handler); }; - }, [dispatch, navigate]); + }, [dispatch, navigate, location]); return null; }; diff --git a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx index fa7ec138ba..75ca10ed94 100644 --- a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx +++ b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx @@ -2,7 +2,12 @@ import React from "react"; import { render, act } from "@testing-library/react"; import { Provider } from "react-redux"; import { combineReducers, configureStore } from "@reduxjs/toolkit"; -import { MemoryRouter, Routes, Route, useLocation } from "react-router-dom"; +import { + MemoryRouter, + Routes, + Route, + useLocation, +} from "react-router-dom"; import browser from "webextension-polyfill"; import { SERVICE_TYPES } from "@shared/constants/services"; @@ -46,19 +51,21 @@ const makeStore = () => } as any, }); -let currentPath = "/"; -const PathSpy = () => { - const location = useLocation(); - currentPath = location.pathname; +let lastLocation: ReturnType | null = null; +const LocationSpy = () => { + lastLocation = useLocation(); return null; }; -const renderListener = (store = makeStore()) => +const renderListener = ( + initialEntry: string = "/", + store = makeStore(), +) => render( - + - + @@ -69,49 +76,87 @@ const renderListener = (store = makeStore()) => describe("SessionLockListener", () => { beforeEach(() => { listeners.length = 0; - currentPath = "/"; + lastLocation = null; jest.clearAllMocks(); }); it("registers a runtime.onMessage listener on mount and removes it on unmount", () => { const { unmount } = renderListener(); - expect(browser.runtime.onMessage.addListener).toHaveBeenCalledTimes(1); - expect(listeners).toHaveLength(1); + expect(browser.runtime.onMessage.addListener).toHaveBeenCalled(); + expect(listeners.length).toBeGreaterThanOrEqual(1); unmount(); - expect(browser.runtime.onMessage.removeListener).toHaveBeenCalledTimes(1); + expect(browser.runtime.onMessage.removeListener).toHaveBeenCalled(); expect(listeners).toHaveLength(0); }); it("dispatches lockAccount and navigates to unlockAccount on SESSION_LOCKED", () => { const store = makeStore(); - renderListener(store); + renderListener("/", store); act(() => { - listeners[0]({ type: SERVICE_TYPES.SESSION_LOCKED }); + listeners[listeners.length - 1]({ + type: SERVICE_TYPES.SESSION_LOCKED, + }); }); const { auth } = store.getState(); expect(auth.hasPrivateKey).toBe(false); expect(auth.publicKey).toBe(""); expect(auth.allAccounts).toEqual([]); - expect(currentPath).toBe(ROUTES.unlockAccount); + expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); + }); + + it("preserves the originating route as state.from and forwards location.search", () => { + const store = makeStore(); + const originalEntry = `${ROUTES.grantAccess}?domain=example.com&public=GA1`; + renderListener(originalEntry, store); + + act(() => { + listeners[listeners.length - 1]({ + type: SERVICE_TYPES.SESSION_LOCKED, + }); + }); + + expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); + expect(lastLocation?.search).toBe("?domain=example.com&public=GA1"); + const state = lastLocation?.state as { from?: { pathname?: string } }; + expect(state?.from?.pathname).toBe(ROUTES.grantAccess); + }); + + it("does not re-navigate or clobber state when already on the unlock screen", () => { + const store = makeStore(); + renderListener(ROUTES.unlockAccount, store); + const initialState = lastLocation?.state; + + act(() => { + listeners[listeners.length - 1]({ + type: SERVICE_TYPES.SESSION_LOCKED, + }); + }); + + // Reducer is not dispatched and no navigation happens. + expect(store.getState().auth.hasPrivateKey).toBe(true); + expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); + expect(lastLocation?.state).toBe(initialState); }); it("ignores unrelated messages", () => { const store = makeStore(); - renderListener(store); + renderListener("/", store); act(() => { - listeners[0]({ type: SERVICE_TYPES.LOAD_ACCOUNT }); - listeners[0]("not an object"); - listeners[0](null); + listeners[listeners.length - 1]({ + type: SERVICE_TYPES.LOAD_ACCOUNT, + }); + listeners[listeners.length - 1]("not an object"); + listeners[listeners.length - 1](null); }); const { auth } = store.getState(); expect(auth.hasPrivateKey).toBe(true); expect(auth.publicKey).toBe("GBTEST"); - expect(currentPath).toBe("/"); + expect(lastLocation?.pathname).toBe("/"); }); }); From 6b22e3a4ade55eaa43e3ac7cec10119b6f788106 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 19:12:09 -0700 Subject: [PATCH 07/21] Address PR review feedback: idle auto-lock cross-surface consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes three correctness bugs in the idle auto-lock flow reported by @leofelix077 on PR #2802: Bug A — Popup stays on Account after auto-lock fires. useGetAppData clearSession deliberately keeps publicKey and only wipes hashKey. Now after the alarm fires lands on the unlock screen. Bug B — clearSession's redux dispatch raced service-worker termination. buildStore's subscribe(saveStore) call was fire-and-forget; when the alarm fired with no popup open, chrome.storage.session.set often didn't complete before SW termination and the next popup read stale hashKey. Fix: - Export flushSessionStore(store) from background/store.ts that explicitly awaits chrome.storage.session.set, and route the subscribe-based save through it too so persistence is unified. - Await flushSessionStore in clearSession (between the dispatches and the TEMPORARY_STORE_ID removal) and in signOut (between dispatch(logOut()) and the TEMPORARY_STORE_ID removal). - Defense-in-depth in buildHasPrivateKeySelector: treat TEMPORARY_STORE_ID absence as authoritative, so a partial lock cannot leave the wallet looking unlocked even if a future flush ever races. Bug C — Unlocking one surface didn't unlock the others. Each surface owns its own redux store, so only the surface that submitted the password saw confirmPassword.fulfilled. Fix: - New SERVICE_TYPES.SESSION_UNLOCKED. - New broadcastSessionState(type) helper (background/messageListener/ helpers/broadcast-session-state.ts) that wraps the browser.runtime.sendMessage + no-receiver swallow pattern. - initAlarmListener replaces its inline try/catch with the helper. - signOut now broadcasts SESSION_LOCKED after the lock work, so explicit sign-out also flips other surfaces' UI. - loginToAllAccounts awaits flushSessionStore then broadcasts SESSION_UNLOCKED on the success path. - SessionLockListener handles SESSION_UNLOCKED: loadAccount + dispatch(saveAccount), and navigate to ROUTES.account only when on ROUTES.unlockAccount or ROUTES.verifyAccount. Does not restore state.from cross-surface (only the surface that entered the password restores its own interrupted flow). Tests: - loadSaveSettings.test.ts — makeLocalStore now seeds TEMPORARY_STORE_ID by default and accepts an opt-out for locked-wallet cases. - SessionLockListener.test.tsx — new SESSION_UNLOCKED cases. - session.test.ts — clearSession awaits flushSessionStore. - signOut.test.ts (new) — signOut awaits flushSessionStore then broadcasts SESSION_LOCKED. - login-all-accounts.test.ts (new) — loginToAllAccounts awaits flushSessionStore then broadcasts SESSION_UNLOCKED on success. yarn test:ci: 983 passed, 72 skipped, 0 failed. yarn build:extension: clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/constants/services.ts | 1 + extension/src/background/ducks/session.ts | 7 +- .../helpers/__tests__/session.test.ts | 58 ++++++++- extension/src/background/helpers/session.ts | 2 + extension/src/background/index.ts | 16 +-- .../__tests__/loadSaveSettings.test.ts | 13 +- .../__tests__/login-all-accounts.test.ts | 116 ++++++++++++++++++ .../messageListener/__tests__/signOut.test.ts | 51 ++++++++ .../messageListener/handlers/signOut.ts | 5 + .../helpers/broadcast-session-state.ts | 13 ++ .../helpers/login-all-accounts.ts | 5 + extension/src/background/store.ts | 20 +-- extension/src/helpers/hooks/useGetAppData.tsx | 1 + .../components/SessionLockListener/index.tsx | 33 +++-- .../__tests__/SessionLockListener.test.tsx | 97 +++++++++++---- 15 files changed, 376 insertions(+), 62 deletions(-) create mode 100644 extension/src/background/messageListener/__tests__/login-all-accounts.test.ts create mode 100644 extension/src/background/messageListener/__tests__/signOut.test.ts create mode 100644 extension/src/background/messageListener/helpers/broadcast-session-state.ts diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index 7f65e3c984..507df44dbb 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -72,6 +72,7 @@ export enum SERVICE_TYPES { DISMISS_DISCOVER_WELCOME = "DISMISS_DISCOVER_WELCOME", USER_ACTIVITY = "USER_ACTIVITY", SESSION_LOCKED = "SESSION_LOCKED", + SESSION_UNLOCKED = "SESSION_UNLOCKED", } // SIDEBAR_NAVIGATE is a plain string constant (not in an enum) because it is diff --git a/extension/src/background/ducks/session.ts b/extension/src/background/ducks/session.ts index 13b5a2a00e..aea5410407 100644 --- a/extension/src/background/ducks/session.ts +++ b/extension/src/background/ducks/session.ts @@ -10,6 +10,7 @@ import { subscribeAccount as internalSubscribeAccount, } from "background/helpers/account"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; +import { TEMPORARY_STORE_ID } from "constants/localStorageTypes"; export const logIn = createAsyncThunk< UiData, @@ -203,7 +204,11 @@ export const buildHasPrivateKeySelector = (localStore: DataStorageAccess) => if (isHardwareWalletActive && !session?.isHardwareWalletLocked) { return true; } - return !!session?.hashKey?.key; + if (!session?.hashKey?.key) { + return false; + } + const temporaryStore = await localStore.getItem(TEMPORARY_STORE_ID); + return !!temporaryStore && Object.keys(temporaryStore).length > 0; }); export const isHardwareWalletLockedSelector = createSelector( diff --git a/extension/src/background/helpers/__tests__/session.test.ts b/extension/src/background/helpers/__tests__/session.test.ts index f57a9876db..a8e77ce57c 100644 --- a/extension/src/background/helpers/__tests__/session.test.ts +++ b/extension/src/background/helpers/__tests__/session.test.ts @@ -6,8 +6,17 @@ import { SESSION_ALARM_NAME, } from "../session"; import browser from "webextension-polyfill"; -import { AUTO_LOCK_TIMEOUT_MINUTES_ID } from "constants/localStorageTypes"; +import { + AUTO_LOCK_TIMEOUT_MINUTES_ID, + TEMPORARY_STORE_ID, +} from "constants/localStorageTypes"; import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; +import { buildHasPrivateKeySelector } from "background/ducks/session"; +import { getIsHardwareWalletActive } from "background/helpers/account"; + +jest.mock("background/helpers/account", () => ({ + getIsHardwareWalletActive: jest.fn(), +})); describe("session", () => { it("should be able to encrypt and decrypt a string", async () => { @@ -167,3 +176,50 @@ describe("SessionTimer", () => { expect(clearMock).toHaveBeenCalledWith(SESSION_ALARM_NAME); }); }); + +describe("buildHasPrivateKeySelector", () => { + const mockGetIsHardwareWalletActive = + getIsHardwareWalletActive as jest.MockedFunction< + typeof getIsHardwareWalletActive + >; + + beforeEach(() => { + mockGetIsHardwareWalletActive.mockResolvedValue(false); + }); + + it("returns true when the hash key exists and the temporary store is populated", async () => { + const selector = buildHasPrivateKeySelector({ + getItem: jest.fn().mockImplementation((key: string) => { + if (key === TEMPORARY_STORE_ID) { + return Promise.resolve({ "key-id-0": "encrypted-blob" }); + } + return Promise.resolve(null); + }), + } as any); + + await expect( + selector({ session: { hashKey: { key: "hash-key" } } } as any), + ).resolves.toBe(true); + }); + + it("returns false when the hash key exists but the temporary store is empty", async () => { + const selector = buildHasPrivateKeySelector({ + getItem: jest.fn().mockResolvedValue({}), + } as any); + + await expect( + selector({ session: { hashKey: { key: "hash-key" } } } as any), + ).resolves.toBe(false); + }); + + it("returns true for unlocked hardware-wallet sessions", async () => { + mockGetIsHardwareWalletActive.mockResolvedValue(true); + const selector = buildHasPrivateKeySelector({ + getItem: jest.fn().mockResolvedValue(null), + } as any); + + await expect( + selector({ session: { isHardwareWalletLocked: false } } as any), + ).resolves.toBe(true); + }); +}); diff --git a/extension/src/background/helpers/session.ts b/extension/src/background/helpers/session.ts index 7b18198745..90ad68bc0e 100644 --- a/extension/src/background/helpers/session.ts +++ b/extension/src/background/helpers/session.ts @@ -8,6 +8,7 @@ import { timeoutAccountAccess, lockHardwareWallet, } from "../ducks/session"; +import { flushSessionStore } from "../store"; import { DataStorageAccess } from "./dataStorageAccess"; import { AUTO_LOCK_TIMEOUT_MINUTES_ID, @@ -310,5 +311,6 @@ export const clearSession = async ({ // `localStore.isHardwareWalletActive` and is unaffected — so without // this dispatch the idle alarm would be a no-op on HW-only sessions. sessionStore.dispatch(lockHardwareWallet()); + await flushSessionStore(sessionStore); await localStore.remove(TEMPORARY_STORE_ID); }; diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index d21f4baa56..0709dc4cfe 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -29,6 +29,7 @@ import { SIDEBAR_DISCONNECT_DEBOUNCE_MS, } from "./helpers/queueCleanup"; import { removeUuidFromAllQueues } from "./messageListener/handlers/rejectSigningRequest"; +import { broadcastSessionState } from "./messageListener/helpers/broadcast-session-state"; import { SESSION_ALARM_NAME, SessionTimer, @@ -231,20 +232,7 @@ export const initAlarmListener = () => { if (name === SESSION_ALARM_NAME) { await clearSession({ sessionStore, localStore }); - // Broadcast to every open extension UI (popup, sidebar, standalone - // signing/grant-access windows) that the wallet just auto-locked, so - // they can flip to the unlock screen instead of leaving stale, - // privilege-suggestive views (Account, Assets, History) on screen - // until the next data refetch. `runtime.sendMessage` only reaches - // extension contexts other than the sender; when no UI is open it - // rejects with "Could not establish connection", which is harmless. - try { - await browser.runtime.sendMessage({ - type: SERVICE_TYPES.SESSION_LOCKED, - }); - } catch { - // No receivers — nothing to do. - } + await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); } }); }; diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts index 3d63adb689..40bfdd7d1d 100644 --- a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -3,6 +3,7 @@ import { saveSettings } from "../handlers/saveSettings"; import { AUTO_LOCK_TIMEOUT_MINUTES_ID, IS_OPEN_SIDEBAR_BY_DEFAULT_ID, + TEMPORARY_STORE_ID, } from "constants/localStorageTypes"; import { DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES } from "@shared/constants/autoLock"; import browser from "webextension-polyfill"; @@ -183,13 +184,21 @@ describe("saveSettings autoLockTimeoutMinutes", () => { stopSession: jest.fn().mockResolvedValue(undefined), }) as any; - const makeLocalStore = (storedTimeout: number | null = 15) => + const makeLocalStore = ( + storedTimeout: number | null = 15, + hasTempStore: boolean = true, + ) => ({ getItem: jest.fn().mockImplementation((key: string) => { if (key === AUTO_LOCK_TIMEOUT_MINUTES_ID) return Promise.resolve(storedTimeout); if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) return Promise.resolve(false); + if (key === TEMPORARY_STORE_ID) { + return Promise.resolve( + hasTempStore ? { "key-id-0": "encrypted-blob" } : null, + ); + } return Promise.resolve(null); }), setItem: jest.fn().mockResolvedValue(undefined), @@ -249,7 +258,7 @@ describe("saveSettings autoLockTimeoutMinutes", () => { const sessionTimer = makeSessionTimer(); await saveSettings({ request: { ...baseRequest, autoLockTimeoutMinutes: 5 } as any, - localStore: makeLocalStore(5), + localStore: makeLocalStore(5, false), sessionStore: makeSessionStore(null), sessionTimer, }); diff --git a/extension/src/background/messageListener/__tests__/login-all-accounts.test.ts b/extension/src/background/messageListener/__tests__/login-all-accounts.test.ts new file mode 100644 index 0000000000..e8b13dc4d2 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/login-all-accounts.test.ts @@ -0,0 +1,116 @@ +import { SERVICE_TYPES } from "@shared/constants/services"; +import { KEY_ID, TEMPORARY_STORE_ID } from "constants/localStorageTypes"; + +import { loginToAllAccounts } from "../helpers/login-all-accounts"; + +const mockGetKeyIdList = jest.fn(); +const mockGetIsHardwareWalletActive = jest.fn(); +const mockUnlockKeystore = jest.fn(); +const mockGetStoredAccounts = jest.fn(); +const mockClearSession = jest.fn().mockResolvedValue(undefined); +const mockDeriveKeyFromString = jest.fn(); +const mockStoreEncryptedTemporaryData = jest.fn().mockResolvedValue(undefined); +const mockStoreActiveHashKey = jest.fn().mockResolvedValue(undefined); +const mockFlushSessionStore = jest.fn().mockResolvedValue(undefined); +const mockBroadcastSessionState = jest.fn().mockResolvedValue(undefined); + +jest.mock("background/helpers/account", () => ({ + HW_PREFIX: "hw:", + getIsHardwareWalletActive: (...args: unknown[]) => + mockGetIsHardwareWalletActive(...args), + getKeyIdList: (...args: unknown[]) => mockGetKeyIdList(...args), +})); + +jest.mock("../helpers/unlock-keystore", () => ({ + unlockKeystore: (...args: unknown[]) => mockUnlockKeystore(...args), +})); + +jest.mock("../helpers/get-stored-accounts", () => ({ + getStoredAccounts: (...args: unknown[]) => mockGetStoredAccounts(...args), +})); + +jest.mock("background/helpers/session", () => ({ + SessionTimer: jest.fn(), + clearSession: (...args: unknown[]) => mockClearSession(...args), + deriveKeyFromString: (...args: unknown[]) => mockDeriveKeyFromString(...args), + storeActiveHashKey: (...args: unknown[]) => mockStoreActiveHashKey(...args), + storeEncryptedTemporaryData: (...args: unknown[]) => + mockStoreEncryptedTemporaryData(...args), +})); + +jest.mock("background/store", () => ({ + flushSessionStore: (...args: unknown[]) => mockFlushSessionStore(...args), +})); + +jest.mock("../helpers/broadcast-session-state", () => ({ + broadcastSessionState: (...args: unknown[]) => + mockBroadcastSessionState(...args), +})); + +jest.mock("@sentry/browser", () => ({ + captureException: jest.fn(), +})); + +describe("loginToAllAccounts", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockGetKeyIdList.mockResolvedValue(["key-id-0"]); + mockGetIsHardwareWalletActive.mockResolvedValue(false); + mockUnlockKeystore + .mockResolvedValueOnce({ + publicKey: "GBACTIVE", + extra: { mnemonicPhrase: "mnemonic phrase" }, + }) + .mockResolvedValueOnce({ + privateKey: "SSECRET", + }); + mockGetStoredAccounts.mockResolvedValue([{ publicKey: "GBACTIVE" }]); + mockDeriveKeyFromString.mockResolvedValue({ key: "derived-key" }); + }); + + it("flushes persisted session state before broadcasting SESSION_UNLOCKED", async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === KEY_ID) { + return Promise.resolve("key-id-0"); + } + return Promise.resolve(null); + }), + remove: jest.fn().mockImplementation((key: string) => { + if (key === TEMPORARY_STORE_ID) { + return Promise.resolve(undefined); + } + return Promise.resolve(undefined); + }), + } as any; + const sessionStore = { + dispatch: jest.fn().mockResolvedValue(undefined), + getState: jest.fn().mockReturnValue({ + session: { + publicKey: "", + allAccounts: [], + }, + }), + } as any; + const keyManager = {} as any; + const sessionTimer = { + startSession: jest.fn().mockResolvedValue(undefined), + } as any; + + await loginToAllAccounts( + "password", + localStore, + sessionStore, + keyManager, + sessionTimer, + ); + + expect(mockFlushSessionStore).toHaveBeenCalledWith(sessionStore); + expect(mockBroadcastSessionState).toHaveBeenCalledWith( + SERVICE_TYPES.SESSION_UNLOCKED, + ); + expect( + mockFlushSessionStore.mock.invocationCallOrder[0], + ).toBeLessThan(mockBroadcastSessionState.mock.invocationCallOrder[0]); + }); +}); diff --git a/extension/src/background/messageListener/__tests__/signOut.test.ts b/extension/src/background/messageListener/__tests__/signOut.test.ts new file mode 100644 index 0000000000..7c1c85f61d --- /dev/null +++ b/extension/src/background/messageListener/__tests__/signOut.test.ts @@ -0,0 +1,51 @@ +import { SERVICE_TYPES } from "@shared/constants/services"; + +import { signOut } from "../handlers/signOut"; + +const mockFlushSessionStore = jest.fn().mockResolvedValue(undefined); +const mockBroadcastSessionState = jest.fn().mockResolvedValue(undefined); + +jest.mock("background/store", () => ({ + flushSessionStore: (...args: unknown[]) => mockFlushSessionStore(...args), +})); + +jest.mock("background/messageListener/helpers/broadcast-session-state", () => ({ + broadcastSessionState: (...args: unknown[]) => + mockBroadcastSessionState(...args), +})); + +describe("signOut handler", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("flushes the session store before clearing temporary storage and broadcasting the lock", async () => { + const localStore = { + getItem: jest.fn().mockResolvedValue("MNEMONIC_PHRASE_CONFIRMED"), + remove: jest.fn().mockResolvedValue(undefined), + } as any; + const sessionStore = { + dispatch: jest.fn(), + getState: jest.fn().mockReturnValue({ + session: { publicKey: "" }, + }), + } as any; + const sessionTimer = { + stopSession: jest.fn().mockResolvedValue(undefined), + } as any; + + await signOut({ localStore, sessionStore, sessionTimer }); + + expect(mockFlushSessionStore).toHaveBeenCalledWith(sessionStore); + expect(localStore.remove).toHaveBeenCalledTimes(1); + expect(mockBroadcastSessionState).toHaveBeenCalledWith( + SERVICE_TYPES.SESSION_LOCKED, + ); + expect( + mockFlushSessionStore.mock.invocationCallOrder[0], + ).toBeLessThan(localStore.remove.mock.invocationCallOrder[0]); + expect( + mockBroadcastSessionState.mock.invocationCallOrder[0], + ).toBeGreaterThan(mockFlushSessionStore.mock.invocationCallOrder[0]); + }); +}); diff --git a/extension/src/background/messageListener/handlers/signOut.ts b/extension/src/background/messageListener/handlers/signOut.ts index 410a7ece6b..0ec65379bf 100644 --- a/extension/src/background/messageListener/handlers/signOut.ts +++ b/extension/src/background/messageListener/handlers/signOut.ts @@ -1,8 +1,11 @@ import { Store } from "redux"; +import { SERVICE_TYPES } from "@shared/constants/services"; import { logOut, publicKeySelector } from "background/ducks/session"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { SessionTimer } from "background/helpers/session"; +import { broadcastSessionState } from "../helpers/broadcast-session-state"; +import { flushSessionStore } from "background/store"; import { APPLICATION_ID, TEMPORARY_STORE_ID, @@ -18,10 +21,12 @@ export const signOut = async ({ sessionTimer: SessionTimer; }) => { sessionStore.dispatch(logOut()); + await flushSessionStore(sessionStore); await localStore.remove(TEMPORARY_STORE_ID); // Cancel any pending auto-lock alarm — the wallet is being locked // explicitly, so the idle timer no longer needs to fire. await sessionTimer.stopSession(); + await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); return { publicKey: publicKeySelector(sessionStore.getState()), diff --git a/extension/src/background/messageListener/helpers/broadcast-session-state.ts b/extension/src/background/messageListener/helpers/broadcast-session-state.ts new file mode 100644 index 0000000000..3ee59a392f --- /dev/null +++ b/extension/src/background/messageListener/helpers/broadcast-session-state.ts @@ -0,0 +1,13 @@ +import browser from "webextension-polyfill"; + +import { SERVICE_TYPES } from "@shared/constants/services"; + +export const broadcastSessionState = async ( + type: SERVICE_TYPES.SESSION_LOCKED | SERVICE_TYPES.SESSION_UNLOCKED, +): Promise => { + try { + await browser.runtime.sendMessage({ type }); + } catch { + // No receivers — harmless. + } +}; diff --git a/extension/src/background/messageListener/helpers/login-all-accounts.ts b/extension/src/background/messageListener/helpers/login-all-accounts.ts index 89181a614f..47794b6e43 100644 --- a/extension/src/background/messageListener/helpers/login-all-accounts.ts +++ b/extension/src/background/messageListener/helpers/login-all-accounts.ts @@ -21,6 +21,7 @@ import { storeActiveHashKey, storeEncryptedTemporaryData, } from "background/helpers/session"; +import { flushSessionStore } from "background/store"; import { allAccountsSelector, logIn, @@ -28,6 +29,8 @@ import { } from "background/ducks/session"; import { getStoredAccounts } from "./get-stored-accounts"; import { captureException } from "@sentry/browser"; +import { SERVICE_TYPES } from "@shared/constants/services"; +import { broadcastSessionState } from "./broadcast-session-state"; /* Retrive and store encrypted data for all existing accounts */ export const loginToAllAccounts = async ( @@ -136,4 +139,6 @@ export const loginToAllAccounts = async ( // start the timer now that we have active private key await sessionTimer.startSession(); + await flushSessionStore(sessionStore); + await broadcastSessionState(SERVICE_TYPES.SESSION_UNLOCKED); }; diff --git a/extension/src/background/store.ts b/extension/src/background/store.ts index 9d6e18ac70..94b7e6936b 100644 --- a/extension/src/background/store.ts +++ b/extension/src/background/store.ts @@ -1,4 +1,4 @@ -import { combineReducers } from "redux"; +import { combineReducers, Store } from "redux"; import { configureStore } from "@reduxjs/toolkit"; import { sessionSlice } from "background/ducks/session"; @@ -29,11 +29,15 @@ const rootReducer = combineReducers({ session: sessionSlice.reducer, }); -type RootState = ReturnType; - -function saveStore(state: RootState) { - const serializedState = JSON.stringify(state); - sessionStore.setItem(REDUX_STORE_KEY, serializedState); +export async function flushSessionStore(storeToFlush: Store): Promise { + try { + const serializedState = JSON.stringify(storeToFlush.getState()); + await sessionStore.setItem(REDUX_STORE_KEY, serializedState); + } catch (_error) { + // Best-effort only: Firefox and some test environments do not expose + // storage.session, and the existing subscribe-based persistence also + // ignores those failures. + } } const store = configureStore({ @@ -56,7 +60,9 @@ export const buildStore = async () => { preloadedState: reduxState, }); - hydratedStore.subscribe(() => saveStore(hydratedStore.getState())); + hydratedStore.subscribe(() => { + void flushSessionStore(hydratedStore); + }); return hydratedStore; } diff --git a/extension/src/helpers/hooks/useGetAppData.tsx b/extension/src/helpers/hooks/useGetAppData.tsx index febaf398da..43fa93d08f 100644 --- a/extension/src/helpers/hooks/useGetAppData.tsx +++ b/extension/src/helpers/hooks/useGetAppData.tsx @@ -74,6 +74,7 @@ function useGetAppData() { if ( !account.publicKey || + !account.hasPrivateKey || account.applicationState === APPLICATION_STATE.APPLICATION_STARTED ) { const hasOnboarded = diff --git a/extension/src/popup/components/SessionLockListener/index.tsx b/extension/src/popup/components/SessionLockListener/index.tsx index e2d9e297dd..1d2cdb1900 100644 --- a/extension/src/popup/components/SessionLockListener/index.tsx +++ b/extension/src/popup/components/SessionLockListener/index.tsx @@ -3,9 +3,10 @@ import { useDispatch } from "react-redux"; import { useLocation, useNavigate } from "react-router-dom"; import browser from "webextension-polyfill"; +import { loadAccount } from "@shared/api/internal"; import { SERVICE_TYPES } from "@shared/constants/services"; import { ROUTES } from "popup/constants/routes"; -import { lockAccount } from "popup/ducks/accountServices"; +import { lockAccount, saveAccount } from "popup/ducks/accountServices"; /** * Listens for the background's `SESSION_LOCKED` broadcast (fired when @@ -32,17 +33,29 @@ export const SessionLockListener = () => { const location = useLocation(); useEffect(() => { - const handler = (message: unknown) => { + const handler = async (message: unknown) => { if (typeof message !== "object" || message === null) return undefined; const { type } = message as { type?: unknown }; - if (type !== SERVICE_TYPES.SESSION_LOCKED) return undefined; - // Already on the unlock screen — nothing to do. Avoids clobbering - // an existing `state.from` set by an earlier reroute. - if (location.pathname === ROUTES.unlockAccount) return undefined; - dispatch(lockAccount()); - navigate(`${ROUTES.unlockAccount}${location.search}`, { - state: { from: location }, - }); + if (type === SERVICE_TYPES.SESSION_LOCKED) { + // Already on the unlock screen — nothing to do. Avoids clobbering + // an existing `state.from` set by an earlier reroute. + if (location.pathname === ROUTES.unlockAccount) return undefined; + dispatch(lockAccount()); + navigate(`${ROUTES.unlockAccount}${location.search}`, { + state: { from: location }, + }); + return undefined; + } + if (type !== SERVICE_TYPES.SESSION_UNLOCKED) return undefined; + + const account = await loadAccount(); + dispatch(saveAccount(account)); + if ( + location.pathname === ROUTES.unlockAccount || + location.pathname === ROUTES.verifyAccount + ) { + navigate(ROUTES.account, { replace: true }); + } return undefined; }; diff --git a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx index 75ca10ed94..4eb3cdb932 100644 --- a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx +++ b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx @@ -10,14 +10,16 @@ import { } from "react-router-dom"; import browser from "webextension-polyfill"; +import { loadAccount } from "@shared/api/internal"; import { SERVICE_TYPES } from "@shared/constants/services"; import { ROUTES } from "popup/constants/routes"; import { reducer as authReducer } from "popup/ducks/accountServices"; import { SessionLockListener } from "../SessionLockListener"; -type RuntimeHandler = (message: unknown) => void; +type RuntimeHandler = (message: unknown) => void | Promise; const listeners: RuntimeHandler[] = []; +const mockLoadAccount = loadAccount as jest.MockedFunction; jest.mock("webextension-polyfill", () => ({ runtime: { @@ -31,6 +33,10 @@ jest.mock("webextension-polyfill", () => ({ }, })); +jest.mock("@shared/api/internal", () => ({ + loadAccount: jest.fn(), +})); + const makeStore = () => configureStore({ reducer: combineReducers({ auth: authReducer }), @@ -51,6 +57,15 @@ const makeStore = () => } as any, }); +const loadedAccount = { + allAccounts: [{ publicKey: "GBUPDATED" } as any], + applicationState: "MNEMONIC_PHRASE_CONFIRMED", + bipPath: "m/44'/148'/1'", + hasPrivateKey: true, + publicKey: "GBUPDATED", + tokenIdList: ["token2"], +} as Awaited>; + let lastLocation: ReturnType | null = null; const LocationSpy = () => { lastLocation = useLocation(); @@ -73,11 +88,18 @@ const renderListener = ( , ); +const emitMessage = async (message: unknown) => { + await act(async () => { + await listeners[listeners.length - 1](message); + }); +}; + describe("SessionLockListener", () => { beforeEach(() => { listeners.length = 0; lastLocation = null; jest.clearAllMocks(); + mockLoadAccount.mockResolvedValue(loadedAccount); }); it("registers a runtime.onMessage listener on mount and removes it on unmount", () => { @@ -91,15 +113,11 @@ describe("SessionLockListener", () => { expect(listeners).toHaveLength(0); }); - it("dispatches lockAccount and navigates to unlockAccount on SESSION_LOCKED", () => { + it("dispatches lockAccount and navigates to unlockAccount on SESSION_LOCKED", async () => { const store = makeStore(); renderListener("/", store); - act(() => { - listeners[listeners.length - 1]({ - type: SERVICE_TYPES.SESSION_LOCKED, - }); - }); + await emitMessage({ type: SERVICE_TYPES.SESSION_LOCKED }); const { auth } = store.getState(); expect(auth.hasPrivateKey).toBe(false); @@ -108,16 +126,12 @@ describe("SessionLockListener", () => { expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); }); - it("preserves the originating route as state.from and forwards location.search", () => { + it("preserves the originating route as state.from and forwards location.search", async () => { const store = makeStore(); const originalEntry = `${ROUTES.grantAccess}?domain=example.com&public=GA1`; renderListener(originalEntry, store); - act(() => { - listeners[listeners.length - 1]({ - type: SERVICE_TYPES.SESSION_LOCKED, - }); - }); + await emitMessage({ type: SERVICE_TYPES.SESSION_LOCKED }); expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); expect(lastLocation?.search).toBe("?domain=example.com&public=GA1"); @@ -125,16 +139,12 @@ describe("SessionLockListener", () => { expect(state?.from?.pathname).toBe(ROUTES.grantAccess); }); - it("does not re-navigate or clobber state when already on the unlock screen", () => { + it("does not re-navigate or clobber state when already on the unlock screen", async () => { const store = makeStore(); renderListener(ROUTES.unlockAccount, store); const initialState = lastLocation?.state; - act(() => { - listeners[listeners.length - 1]({ - type: SERVICE_TYPES.SESSION_LOCKED, - }); - }); + await emitMessage({ type: SERVICE_TYPES.SESSION_LOCKED }); // Reducer is not dispatched and no navigation happens. expect(store.getState().auth.hasPrivateKey).toBe(true); @@ -142,17 +152,50 @@ describe("SessionLockListener", () => { expect(lastLocation?.state).toBe(initialState); }); - it("ignores unrelated messages", () => { + it("loads and saves account data on SESSION_UNLOCKED", async () => { + const store = makeStore(); + renderListener("/", store); + + await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); + + expect(mockLoadAccount).toHaveBeenCalledTimes(1); + const { auth } = store.getState(); + expect(auth.hasPrivateKey).toBe(true); + expect(auth.publicKey).toBe("GBUPDATED"); + expect(auth.allAccounts).toEqual([{ publicKey: "GBUPDATED" }]); + }); + + it("navigates to account from unlockAccount on SESSION_UNLOCKED", async () => { + renderListener(ROUTES.unlockAccount); + + await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); + + expect(lastLocation?.pathname).toBe(ROUTES.account); + }); + + it("navigates to account from verifyAccount on SESSION_UNLOCKED", async () => { + renderListener(ROUTES.verifyAccount); + + await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); + + expect(lastLocation?.pathname).toBe(ROUTES.account); + }); + + it("does not navigate from a non-lock route on SESSION_UNLOCKED", async () => { + renderListener(ROUTES.settings); + + await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); + + expect(lastLocation?.pathname).toBe(ROUTES.settings); + }); + + it("ignores unrelated messages", async () => { const store = makeStore(); renderListener("/", store); - act(() => { - listeners[listeners.length - 1]({ - type: SERVICE_TYPES.LOAD_ACCOUNT, - }); - listeners[listeners.length - 1]("not an object"); - listeners[listeners.length - 1](null); - }); + await emitMessage({ type: SERVICE_TYPES.LOAD_ACCOUNT }); + await emitMessage("not an object"); + await emitMessage(null); const { auth } = store.getState(); expect(auth.hasPrivateKey).toBe(true); From f3e4598c1ee4bfdcf2b251af074aa5b3f7732324 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 19:51:52 -0700 Subject: [PATCH 08/21] Address internal review: require hasPrivateKey in useGetAppData cache fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal review-fix round 1 (CONCERNS) noted Bug A was only partially addressed: the post-loadAccount reroute in useGetAppData correctly useGetAppData.tsx:58 short-circuited on currentAccount.publicKey alone. After a locked loadAccount response is saved into Redux (saveAccount), a later cached fetchData could still return RESOLVED for publicKey-present + hasPrivateKey:false and bypass the reroute. Fix: also require currentAccount.hasPrivateKey in the cache fast path, so the publicKey-vs-hasPrivateKey invariant is consistent across both the cached and freshly-loaded paths. Tests: - useGetAppData.test.tsx (new) — direct regression test for Bug A: cached publicKey + hasPrivateKey:false re-routes to unlockAccount; cached publicKey + hasPrivateKey:true short-circuits to RESOLVED. - Backfilled hasPrivateKey: true in five existing test files whose Redux preloadedState seeded an unlocked account with only publicKey: AddFunds.test.tsx, GrantAccess.test.tsx, SearchAsset.test.tsx, useGetWalletsData.test.tsx, useGetAssetDomainsWithBalances.test.tsx. yarn test:ci: 985 passed, 72 skipped, 0 failed. yarn build:extension: clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../helpers/__tests__/useGetAppData.test.tsx | 81 +++++++++++++++++++ .../useGetAssetDomainsWithBalances.test.tsx | 1 + extension/src/helpers/hooks/useGetAppData.tsx | 6 +- .../components/__tests__/SearchAsset.test.tsx | 2 + .../__tests__/useGetWalletsData.test.tsx | 3 + .../popup/views/__tests__/AddFunds.test.tsx | 2 + .../views/__tests__/GrantAccess.test.tsx | 7 ++ 7 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 extension/src/helpers/__tests__/useGetAppData.test.tsx diff --git a/extension/src/helpers/__tests__/useGetAppData.test.tsx b/extension/src/helpers/__tests__/useGetAppData.test.tsx new file mode 100644 index 0000000000..4d4f1965f2 --- /dev/null +++ b/extension/src/helpers/__tests__/useGetAppData.test.tsx @@ -0,0 +1,81 @@ +import React from "react"; +import { Provider } from "react-redux"; +import { renderHook, act } from "@testing-library/react"; + +import { useGetAppData, AppDataType } from "../hooks/useGetAppData"; +import { makeDummyStore } from "popup/__testHelpers__"; +import { APPLICATION_STATE } from "@shared/constants/applicationState"; +import { ROUTES } from "popup/constants/routes"; +import * as ApiInternal from "@shared/api/internal"; + +const renderUseGetAppData = (preloadedAuthState: any) => { + const store = makeDummyStore({ + auth: { + allAccounts: [], + publicKey: preloadedAuthState.publicKey ?? "", + hasPrivateKey: preloadedAuthState.hasPrivateKey ?? false, + applicationState: + preloadedAuthState.applicationState ?? + APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, + accountStatus: "IDLE", + }, + settings: {}, + }); + const Wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + return renderHook(() => useGetAppData(), { wrapper: Wrapper }); +}; + +describe("useGetAppData — cache fast path requires hasPrivateKey (Bug A regression)", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("does NOT short-circuit on cached account when hasPrivateKey is false; re-routes to unlockAccount", async () => { + const loadAccountSpy = jest + .spyOn(ApiInternal, "loadAccount") + .mockResolvedValueOnce({ + publicKey: "GCACHED", + hasPrivateKey: false, + allAccounts: [], + applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, + } as any); + jest.spyOn(ApiInternal, "loadSettings").mockResolvedValueOnce({} as any); + + const { result } = renderUseGetAppData({ + publicKey: "GCACHED", + hasPrivateKey: false, + applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, + }); + + let payload: any; + await act(async () => { + payload = await result.current.fetchData(true); + }); + + // Cache had publicKey but no hasPrivateKey — fast path must NOT short-circuit. + expect(loadAccountSpy).toHaveBeenCalled(); + expect(payload?.type).toBe(AppDataType.REROUTE); + expect(payload?.routeTarget).toBe(ROUTES.unlockAccount); + }); + + it("does short-circuit on cached account when both publicKey and hasPrivateKey are set", async () => { + const loadAccountSpy = jest.spyOn(ApiInternal, "loadAccount"); + + const { result } = renderUseGetAppData({ + publicKey: "GCACHED", + hasPrivateKey: true, + applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, + }); + + let payload: any; + await act(async () => { + payload = await result.current.fetchData(true); + }); + + expect(loadAccountSpy).not.toHaveBeenCalled(); + expect(payload?.type).toBe(AppDataType.RESOLVED); + expect(payload?.account?.publicKey).toBe("GCACHED"); + }); +}); diff --git a/extension/src/helpers/__tests__/useGetAssetDomainsWithBalances.test.tsx b/extension/src/helpers/__tests__/useGetAssetDomainsWithBalances.test.tsx index 9b18836286..57b0ec5c4c 100644 --- a/extension/src/helpers/__tests__/useGetAssetDomainsWithBalances.test.tsx +++ b/extension/src/helpers/__tests__/useGetAssetDomainsWithBalances.test.tsx @@ -78,6 +78,7 @@ describe("useGetAssetDomainsWithBalances (cached path)", () => { const preloadedState = { auth: { publicKey: TEST_PUBLIC_KEY, + hasPrivateKey: true, }, cache: { balanceData: { diff --git a/extension/src/helpers/hooks/useGetAppData.tsx b/extension/src/helpers/hooks/useGetAppData.tsx index 43fa93d08f..37b76efd10 100644 --- a/extension/src/helpers/hooks/useGetAppData.tsx +++ b/extension/src/helpers/hooks/useGetAppData.tsx @@ -55,7 +55,11 @@ function useGetAppData() { dispatch({ type: "FETCH_DATA_START" }); reduxDispatch(saveApplicationState(APPLICATION_STATE.APPLICATION_LOADING)); try { - if (useCache && currentAccount.publicKey) { + if ( + useCache && + currentAccount.publicKey && + currentAccount.hasPrivateKey + ) { const payload = { type: "resolved", account: currentAccount, diff --git a/extension/src/popup/components/__tests__/SearchAsset.test.tsx b/extension/src/popup/components/__tests__/SearchAsset.test.tsx index 75a1fda3ff..27b84cd865 100644 --- a/extension/src/popup/components/__tests__/SearchAsset.test.tsx +++ b/extension/src/popup/components/__tests__/SearchAsset.test.tsx @@ -32,6 +32,7 @@ describe("SearchAsset", () => { error: null, applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "G1", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -112,6 +113,7 @@ describe("SearchAsset", () => { error: null, applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "G1", + hasPrivateKey: true, allAccounts: mockAccounts, balances: mockBalances, }, diff --git a/extension/src/popup/views/Wallets/hooks/__tests__/useGetWalletsData.test.tsx b/extension/src/popup/views/Wallets/hooks/__tests__/useGetWalletsData.test.tsx index 0c678ee3b9..eab97eb26d 100644 --- a/extension/src/popup/views/Wallets/hooks/__tests__/useGetWalletsData.test.tsx +++ b/extension/src/popup/views/Wallets/hooks/__tests__/useGetWalletsData.test.tsx @@ -66,6 +66,7 @@ describe("useGetWalletsData", () => { const preloadedState = { auth: { publicKey: TEST_PUBLIC_KEY, + hasPrivateKey: true, allAccounts: mockAccounts, applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, }, @@ -113,6 +114,7 @@ describe("useGetWalletsData", () => { const mainnetPreloadedState = { auth: { publicKey: "G1", + hasPrivateKey: true, allAccounts: [{ publicKey: "G1" }], applicationState: APPLICATION_STATE.MNEMONIC_PHRASE_CONFIRMED, }, @@ -166,6 +168,7 @@ describe("useGetWalletsData", () => { const mainnetPreloadedState = { auth: { publicKey: "G1", + hasPrivateKey: true, allAccounts: [ { publicKey: "G1" }, { publicKey: "G2" }, diff --git a/extension/src/popup/views/__tests__/AddFunds.test.tsx b/extension/src/popup/views/__tests__/AddFunds.test.tsx index dae7f68c85..8ad606cacf 100644 --- a/extension/src/popup/views/__tests__/AddFunds.test.tsx +++ b/extension/src/popup/views/__tests__/AddFunds.test.tsx @@ -86,6 +86,7 @@ describe("AddFunds view", () => { error: null, applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "G1", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -118,6 +119,7 @@ describe("AddFunds view", () => { error: null, applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "G1", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { diff --git a/extension/src/popup/views/__tests__/GrantAccess.test.tsx b/extension/src/popup/views/__tests__/GrantAccess.test.tsx index e1c2bf1c61..1c830d25b8 100644 --- a/extension/src/popup/views/__tests__/GrantAccess.test.tsx +++ b/extension/src/popup/views/__tests__/GrantAccess.test.tsx @@ -101,6 +101,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -143,6 +144,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.PASSWORD_CREATED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -178,6 +180,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -218,6 +221,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -253,6 +257,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -294,6 +299,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { @@ -349,6 +355,7 @@ describe("Grant Access view", () => { applicationState: ApplicationState.MNEMONIC_PHRASE_CONFIRMED, publicKey: "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF", + hasPrivateKey: true, allAccounts: mockAccounts, }, settings: { From 2013aad4f234b4edb465de186e730ec7aadeb5ac Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 22:43:42 -0700 Subject: [PATCH 09/21] Make SessionLockListener handler synchronous to avoid claiming runtime.onMessage response slot When multiple Freighter UI surfaces are open (popup, sidebar, fullscreen), they each mount a SessionLockListener that registers a runtime.onMessage listener. browser.runtime.sendMessage delivers to every extension context except the sender, so cross-surface requests (LOAD_ACCOUNT, GET_IS_ACCOUNT_MISMATCH, CONFIRM_PASSWORD, etc.) also reach the other surfaces' SessionLockListener handler. The previous handler was an async function, which always returned a Promise. Chrome interprets a Promise return as 'this listener will respond', and that immediately-resolved Promise(undefined) could win the race against the background's real handler. The sender then received undefined/null instead of the real payload, surfacing as: - 'Cannot read properties of null (reading \'publicKey\')' when opening a second surface (the chained appData fetch dereferenced the null account). - 'Cannot read properties of null (reading \'error\')' beneath the password input when entering the password on one surface while another was open (activePublicKeyMiddleware fires getIsAccountMismatch on every pending action; its internal client does 'if (response.error)' which throws on a null response, and the thrown message lands in authError). Fix: the handler is now synchronous. For unrelated message types it returns undefined immediately, leaving the response slot to the background. For SESSION_LOCKED / SESSION_UNLOCKED it still returns undefined synchronously (the background's broadcast doesn't await a reply); the SESSION_UNLOCKED loadAccount round-trip runs as a fire-and-forget side effect. Adds two regression tests asserting the handler returns undefined synchronously for unrelated messages and for the broadcast types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../components/SessionLockListener/index.tsx | 46 +++++++++++++++---- .../__tests__/SessionLockListener.test.tsx | 36 +++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/extension/src/popup/components/SessionLockListener/index.tsx b/extension/src/popup/components/SessionLockListener/index.tsx index 1d2cdb1900..f686f68f06 100644 --- a/extension/src/popup/components/SessionLockListener/index.tsx +++ b/extension/src/popup/components/SessionLockListener/index.tsx @@ -33,9 +33,34 @@ export const SessionLockListener = () => { const location = useLocation(); useEffect(() => { - const handler = async (message: unknown) => { + // IMPORTANT: this handler must be a *synchronous* function. Every + // Freighter UI surface (popup, sidebar, fullscreen) registers a + // `runtime.onMessage` listener, and `browser.runtime.sendMessage` + // broadcasts to every extension context except the sender. If this + // handler were `async`, it would always return a Promise — which + // Chrome interprets as "this listener will produce the response" — + // and could win the race against the background's own handler for + // any unrelated request (e.g. `LOAD_ACCOUNT`, + // `GET_IS_ACCOUNT_MISMATCH`) sent by another surface. The sender + // would then receive `undefined` from this listener instead of the + // real background payload, surfacing as crashes like + // "Cannot read properties of null (reading 'publicKey')". + // + // To stay out of the response slot for unrelated messages we + // return `undefined` synchronously below. For our two broadcast + // types we still don't claim the response slot (the background's + // broadcast doesn't await any reply); the SESSION_UNLOCKED + // `loadAccount` round-trip runs as a fire-and-forget side effect. + const handler = (message: unknown) => { if (typeof message !== "object" || message === null) return undefined; const { type } = message as { type?: unknown }; + if ( + type !== SERVICE_TYPES.SESSION_LOCKED && + type !== SERVICE_TYPES.SESSION_UNLOCKED + ) { + return undefined; + } + if (type === SERVICE_TYPES.SESSION_LOCKED) { // Already on the unlock screen — nothing to do. Avoids clobbering // an existing `state.from` set by an earlier reroute. @@ -46,16 +71,17 @@ export const SessionLockListener = () => { }); return undefined; } - if (type !== SERVICE_TYPES.SESSION_UNLOCKED) return undefined; - const account = await loadAccount(); - dispatch(saveAccount(account)); - if ( - location.pathname === ROUTES.unlockAccount || - location.pathname === ROUTES.verifyAccount - ) { - navigate(ROUTES.account, { replace: true }); - } + void (async () => { + const account = await loadAccount(); + dispatch(saveAccount(account)); + if ( + location.pathname === ROUTES.unlockAccount || + location.pathname === ROUTES.verifyAccount + ) { + navigate(ROUTES.account, { replace: true }); + } + })(); return undefined; }; diff --git a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx index 4eb3cdb932..0a4bccc5bb 100644 --- a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx +++ b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx @@ -94,6 +94,9 @@ const emitMessage = async (message: unknown) => { }); }; +const callListenerSync = (message: unknown) => + listeners[listeners.length - 1](message); + describe("SessionLockListener", () => { beforeEach(() => { listeners.length = 0; @@ -202,4 +205,37 @@ describe("SessionLockListener", () => { expect(auth.publicKey).toBe("GBTEST"); expect(lastLocation?.pathname).toBe("/"); }); + + // Regression: when multiple Freighter surfaces are open, the background + // and every popup-side SessionLockListener share `runtime.onMessage`. + // Returning a Promise (e.g. from an `async` handler) tells Chrome + // "this listener will respond" and can win the race against the + // background handler for unrelated requests, leaking `undefined` to + // the sender. The handler must therefore return `undefined` + // synchronously for messages it does not own. + it("returns undefined synchronously for unrelated messages so it does not claim the response slot", () => { + renderListener(); + + expect(callListenerSync({ type: SERVICE_TYPES.LOAD_ACCOUNT })).toBeUndefined(); + expect( + callListenerSync({ type: SERVICE_TYPES.GET_IS_ACCOUNT_MISMATCH }), + ).toBeUndefined(); + expect(callListenerSync("not an object")).toBeUndefined(); + expect(callListenerSync(null)).toBeUndefined(); + }); + + it("returns undefined synchronously for SESSION_LOCKED / SESSION_UNLOCKED too (background broadcast doesn't await a reply)", () => { + renderListener("/", makeStore()); + + let lockedResult: unknown; + let unlockedResult: unknown; + act(() => { + lockedResult = callListenerSync({ type: SERVICE_TYPES.SESSION_LOCKED }); + unlockedResult = callListenerSync({ + type: SERVICE_TYPES.SESSION_UNLOCKED, + }); + }); + expect(lockedResult).toBeUndefined(); + expect(unlockedResult).toBeUndefined(); + }); }); From ece59710cd0fb2f5a44feeefb2f94428e6cd8893 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 23:03:42 -0700 Subject: [PATCH 10/21] Move post-unlock navigation from SessionLockListener to UnlockAccount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SessionLockListener was navigating to /account on SESSION_UNLOCKED from the /unlock-account route. Because runtime.sendMessage broadcasts to every extension context — including the popup that initiated the unlock via confirmPassword — this navigation raced the UnlockAccount post-submit navigation. In the grant-access flow that meant the popup sometimes landed on /account instead of the /grant-access destination preserved via state.from, which broke the freighterApiIntegration.test.ts "should get public key when logged out" e2e test. Move navigation responsibility to UnlockAccount itself: it watches hasPrivateKey in redux and navigates to from || /account on the false → true transition. This unifies both surfaces — the one that submitted the password and the passive surface that received the cross-surface SESSION_UNLOCKED broadcast — through a single code path and removes the race. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../components/SessionLockListener/index.tsx | 17 +++++++----- .../__tests__/SessionLockListener.test.tsx | 12 ++------- .../src/popup/views/UnlockAccount/index.tsx | 26 ++++++++++++++----- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/extension/src/popup/components/SessionLockListener/index.tsx b/extension/src/popup/components/SessionLockListener/index.tsx index f686f68f06..e5b3375125 100644 --- a/extension/src/popup/components/SessionLockListener/index.tsx +++ b/extension/src/popup/components/SessionLockListener/index.tsx @@ -72,15 +72,20 @@ export const SessionLockListener = () => { return undefined; } + // SESSION_UNLOCKED. Refresh this surface's auth state so passive + // surfaces (e.g. sidebar parked on /unlock-account) reflect the + // unlocked wallet. We deliberately do NOT navigate here: + // `runtime.sendMessage` is broadcast by the background to every + // extension context including the popup that *initiated* the + // unlock via `confirmPassword`, so navigating from this listener + // would race the unlock view's own post-submit navigation (and + // in the grant-access flow would land the user on /account + // instead of /grant-access). The unlock screens watch + // `hasPrivateKey` and navigate themselves once auth state + // flips, which covers both the active and passive surfaces. void (async () => { const account = await loadAccount(); dispatch(saveAccount(account)); - if ( - location.pathname === ROUTES.unlockAccount || - location.pathname === ROUTES.verifyAccount - ) { - navigate(ROUTES.account, { replace: true }); - } })(); return undefined; }; diff --git a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx index 0a4bccc5bb..8f622e17cb 100644 --- a/extension/src/popup/components/__tests__/SessionLockListener.test.tsx +++ b/extension/src/popup/components/__tests__/SessionLockListener.test.tsx @@ -168,20 +168,12 @@ describe("SessionLockListener", () => { expect(auth.allAccounts).toEqual([{ publicKey: "GBUPDATED" }]); }); - it("navigates to account from unlockAccount on SESSION_UNLOCKED", async () => { + it("does not navigate from any route on SESSION_UNLOCKED (unlock views navigate themselves via redux state)", async () => { renderListener(ROUTES.unlockAccount); await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); - expect(lastLocation?.pathname).toBe(ROUTES.account); - }); - - it("navigates to account from verifyAccount on SESSION_UNLOCKED", async () => { - renderListener(ROUTES.verifyAccount); - - await emitMessage({ type: SERVICE_TYPES.SESSION_UNLOCKED }); - - expect(lastLocation?.pathname).toBe(ROUTES.account); + expect(lastLocation?.pathname).toBe(ROUTES.unlockAccount); }); it("does not navigate from a non-lock route on SESSION_UNLOCKED", async () => { diff --git a/extension/src/popup/views/UnlockAccount/index.tsx b/extension/src/popup/views/UnlockAccount/index.tsx index 359c092ece..d0f4c71b7d 100644 --- a/extension/src/popup/views/UnlockAccount/index.tsx +++ b/extension/src/popup/views/UnlockAccount/index.tsx @@ -1,8 +1,8 @@ import { Button } from "@stellar/design-system"; import get from "lodash/get"; -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; -import { useDispatch } from "react-redux"; +import { useDispatch, useSelector } from "react-redux"; import { useLocation, useNavigate } from "react-router-dom"; import { newTabHref } from "helpers/urls"; @@ -12,6 +12,7 @@ import { openTab } from "popup/helpers/navigate"; import { View } from "popup/basics/layout/View"; import { confirmPassword, + hasPrivateKeySelector, loadLastUsedAccount, } from "popup/ducks/accountServices"; import { EnterPassword } from "popup/components/EnterPassword"; @@ -30,14 +31,27 @@ export const UnlockAccount = () => { const [accountAddress, setAccountAddress] = useState(""); const dispatch = useDispatch(); + const hasPrivateKey = useSelector(hasPrivateKeySelector); + // Track the initial auth state so we don't auto-navigate if the + // component happens to mount while the wallet is already unlocked + // (e.g. a stale `/unlock-account` route). We only redirect on the + // false → true transition that signals an unlock just happened + // (either via this surface's password submit or via a cross-surface + // SESSION_UNLOCKED broadcast saving fresh auth state). + const wasLockedOnMount = useRef(!hasPrivateKey); const handleSubmit = async (password: string) => { - const res = await dispatch(confirmPassword(password)); - if (confirmPassword.fulfilled.match(res) && res.payload.publicKey) { - // skip this location in history, we won't need to come back here after unlocking account + await dispatch(confirmPassword(password)); + // Navigation is handled by the `hasPrivateKey` effect below so + // that both password-submit and cross-surface SESSION_UNLOCKED + // broadcasts converge on the same destination. + }; + + useEffect(() => { + if (wasLockedOnMount.current && hasPrivateKey) { navigate(`${destination}${queryParams}`, { replace: true }); } - }; + }, [hasPrivateKey, destination, queryParams, navigate]); useEffect(() => { const fetchLastUsedAccount = async () => { From a5ed79dc36bd92343a10f259280d1df544df64a0 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 26 May 2026 23:27:50 -0700 Subject: [PATCH 11/21] Reset idle alarm when a new Freighter surface mounts unlocked Previously the idle auto-lock alarm only rearmed on user input events (mousedown, keydown, touchstart, wheel). When a dApp triggered a flow that opened a new popup near the end of the idle window, the user could be locked out mid-flow before they ever interacted with the new surface. Ping USER_ACTIVITY once when useActivityPing mounts in the unlocked state. Opening any Freighter surface (popup, sidebar, fullscreen) is itself an act of user intent and is enough signal to treat the user as active. The existing throttle and isUnlocked guard naturally suppress the mount ping when the wallet is locked or when an input event fires within the same throttle window. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../hooks/__tests__/useActivityPing.test.ts | 30 +++++++++++++++++++ .../popup/helpers/hooks/useActivityPing.ts | 25 +++++++++++++--- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts index b140d76d61..c4909aa02d 100644 --- a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts +++ b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts @@ -92,4 +92,34 @@ describe("useActivityPing", () => { addSpy.mockRestore(); }); + + it("sends a USER_ACTIVITY ping on mount when unlocked", () => { + renderHook(() => useActivityPing(true)); + + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + expect(sendMessageToBackground).toHaveBeenCalledWith({ + type: SERVICE_TYPES.USER_ACTIVITY, + activePublicKey: "", + }); + }); + + it("throttles event-driven pings against the mount ping", () => { + renderHook(() => useActivityPing(true)); + + // Mount ping has already fired at t=10_000; event within the + // throttle window is suppressed. + window.dispatchEvent(new MouseEvent("mousedown")); + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + + // Past the throttle window, events resume firing. + jest.setSystemTime(15_000); + window.dispatchEvent(new MouseEvent("mousedown")); + expect(sendMessageToBackground).toHaveBeenCalledTimes(2); + }); + + it("does not ping on mount when locked", () => { + renderHook(() => useActivityPing(false)); + + expect(sendMessageToBackground).not.toHaveBeenCalled(); + }); }); diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index b88d2ea3e7..d8432c0555 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -23,16 +23,19 @@ const ACTIVITY_EVENTS = [ * Uses a leading-edge throttle so a single ping fires on the first * event in each `PING_THROTTLE_MS` window — accurate to ~8 % on the * 1-minute preset and effectively noise at higher presets. + * + * Also pings once on mount, so opening a new Freighter surface (popup, + * sidebar, fullscreen) while the wallet is unlocked itself counts as + * activity. This prevents the wallet from auto-locking mid-flow when a + * dApp triggers a new popup near the end of the idle window. */ export const useActivityPing = (isUnlocked: boolean) => { useEffect(() => { if (!isUnlocked) return undefined; let lastPingAt = 0; - const handler = () => { - const now = Date.now(); - if (now - lastPingAt < PING_THROTTLE_MS) return; - lastPingAt = now; + const ping = () => { + lastPingAt = Date.now(); void sendMessageToBackground({ type: SERVICE_TYPES.USER_ACTIVITY, // `USER_ACTIVITY` is account-agnostic — the background only @@ -46,6 +49,20 @@ export const useActivityPing = (isUnlocked: boolean) => { }); }; + // Ping once on mount so opening a fresh Freighter surface (popup, + // sidebar, fullscreen) while the wallet is unlocked counts as + // activity and rearms the idle alarm. Without this, a user who + // triggers a dApp flow that opens a new popup near the end of the + // idle window can be locked out mid-flow before they ever interact + // with the new surface. + ping(); + + const handler = () => { + const now = Date.now(); + if (now - lastPingAt < PING_THROTTLE_MS) return; + ping(); + }; + for (const evt of ACTIVITY_EVENTS) { window.addEventListener(evt, handler, { passive: true }); } From b3868f9c1ecc70036526496d7cd7c9b4bed0d766 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 02:07:53 -0700 Subject: [PATCH 12/21] Fire surface-mount activity ping unconditionally; gate on lock state in background MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous mount ping in useActivityPing was gated on the popup-side isUnlocked signal, which is read from redux and only flips true after loadAccount roundtrips to the background and dispatches saveAccount. On a freshly-spawned dApp popup that gate is racy at best and silently fails to fire at worst — users still saw the wallet auto-lock partway through a signing flow because the new surface never managed to rearm the alarm before the previous deadline elapsed. Fix it by separating the two responsibilities: - The popup fires the mount ping unconditionally, exactly once per surface mount, in its own dependency-less useEffect. Opening any Freighter UI is itself a user-initiated action. - The background's userActivity handler is now the authoritative gate: it reads hashKey and isHardwareWalletActive directly from its own state and only rearms the idle alarm when the wallet is actually unlocked (hot or HW). A ping that arrives on a locked wallet is dropped — there is no live session to extend. This matches the user's stated requirement ('reset the timer when a new instance opens, as long as Freighter is currently unlocked') while removing the dependency on a redux signal that the popup cannot guarantee has hydrated by the time the ping needs to fire. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../__tests__/userActivity.test.ts | 91 +++++++++++++++++-- .../messageListener/handlers/userActivity.ts | 49 +++++++--- .../messageListener/popupMessageListener.ts | 2 +- .../hooks/__tests__/useActivityPing.test.ts | 13 ++- .../popup/helpers/hooks/useActivityPing.ts | 66 ++++++++------ 5 files changed, 170 insertions(+), 51 deletions(-) diff --git a/extension/src/background/messageListener/__tests__/userActivity.test.ts b/extension/src/background/messageListener/__tests__/userActivity.test.ts index 1e40e83cdf..4d17bd3cce 100644 --- a/extension/src/background/messageListener/__tests__/userActivity.test.ts +++ b/extension/src/background/messageListener/__tests__/userActivity.test.ts @@ -1,5 +1,11 @@ import { userActivity } from "../handlers/userActivity"; +const mockGetIsHardwareWalletActive = jest.fn(); +jest.mock("background/helpers/account", () => ({ + getIsHardwareWalletActive: (...args: unknown[]) => + mockGetIsHardwareWalletActive(...args), +})); + const makeSessionTimer = () => ({ resetSession: jest.fn().mockResolvedValue(undefined), @@ -7,16 +13,87 @@ const makeSessionTimer = () => stopSession: jest.fn().mockResolvedValue(undefined), }) as any; +const makeSessionStore = (state: any) => + ({ + getState: () => state, + }) as any; + +const makeLocalStore = () => ({}) as any; + describe("userActivity handler", () => { - it("resets the session timer on every ping", async () => { - // The caller-side guards (popup-side `useActivityPing` only attaches - // listeners while unlocked, and `popupMessageListener` gates this - // message behind `isFromExtensionPage`) mean the handler is reached - // only on genuine user activity. The handler itself unconditionally - // rearms the idle alarm. + beforeEach(() => { + mockGetIsHardwareWalletActive.mockReset(); + }); + + it("resets the timer when the hot wallet is unlocked", 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: true }); + expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); + }); + + it("resets the timer when a hardware wallet is active and unlocked", async () => { + mockGetIsHardwareWalletActive.mockResolvedValue(true); const sessionTimer = makeSessionTimer(); - const result = await userActivity({ sessionTimer }); + const sessionStore = makeSessionStore({ + session: { + hashKey: { key: "" }, + isHardwareWalletLocked: false, + }, + }); + const result = await userActivity({ + sessionTimer, + sessionStore, + localStore: makeLocalStore(), + }); expect(result).toEqual({ ok: true }); expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); }); + + it("does NOT reset the timer when the wallet is locked", async () => { + mockGetIsHardwareWalletActive.mockResolvedValue(false); + const sessionTimer = makeSessionTimer(); + const sessionStore = makeSessionStore({ + session: { + hashKey: { key: "" }, + isHardwareWalletLocked: false, + }, + }); + const result = await userActivity({ + sessionTimer, + sessionStore, + localStore: makeLocalStore(), + }); + expect(result).toEqual({ ok: false }); + expect(sessionTimer.resetSession).not.toHaveBeenCalled(); + }); + + it("does NOT reset the timer when a hardware wallet is active but locked", async () => { + mockGetIsHardwareWalletActive.mockResolvedValue(true); + const sessionTimer = makeSessionTimer(); + const sessionStore = makeSessionStore({ + session: { + hashKey: { key: "" }, + isHardwareWalletLocked: true, + }, + }); + 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 006a579907..358730ecec 100644 --- a/extension/src/background/messageListener/handlers/userActivity.ts +++ b/extension/src/background/messageListener/handlers/userActivity.ts @@ -1,26 +1,53 @@ +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, + SessionState, +} from "background/ducks/session"; /** * Handle a USER_ACTIVITY ping from an extension page. * - * Rearms the idle auto-lock alarm by delegating to - * `sessionTimer.resetSession()`. The popup-side `useActivityPing` hook - * only attaches event listeners while the wallet is unlocked, and - * `popupMessageListener` gates this message behind `isFromExtensionPage` - * so dApp content scripts cannot reach it — so a ping arriving here is - * already proof of genuine user activity in an unlocked extension page. + * Pings come from two sources inside `useActivityPing`: + * 1. An unconditional mount ping fired by every Freighter surface + * (popup, sidebar, fullscreen) as it loads. + * 2. Throttled user-input events while the popup believes the wallet + * is unlocked. + * + * Either way, the popup-side `isUnlocked` is a delayed reflection of + * the background's session state (it depends on `loadAccount` having + * dispatched `saveAccount` into the popup's redux store). The + * background is the source of truth, so this handler authoritatively + * checks whether the wallet is currently unlocked before rearming the + * idle alarm. A ping that arrives on a locked wallet is dropped: there + * is no live session to extend. * - * In the unlikely race where the wallet locks between the user's input - * and the popup tearing down its listeners, an extra `resetSession()` - * just (re)schedules an alarm that will fire on a locked session and - * be a no-op when `clearSession` runs against state that's already - * cleared. + * Unlocked = either a hot-wallet session (`hashKey` is set) or an + * active hardware-wallet session that has not been idle-locked. */ export const userActivity = async ({ sessionTimer, + sessionStore, + localStore, }: { sessionTimer: SessionTimer; + 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); + + if (!hotUnlocked && !hwUnlocked) { + return { ok: false }; + } + await sessionTimer.resetSession(); return { ok: true }; }; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index e549321b31..e3dc8c9dcb 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -631,7 +631,7 @@ export const popupMessageListener = ( case SERVICE_TYPES.USER_ACTIVITY: { if (!isFromExtensionPage) return { error: "Unauthorized" }; - return userActivity({ sessionTimer }); + return userActivity({ sessionTimer, sessionStore, localStore }); } default: diff --git a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts index c4909aa02d..0ea2e627db 100644 --- a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts +++ b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts @@ -88,7 +88,10 @@ describe("useActivityPing", () => { expect.anything(), ); } - expect(sendMessageToBackground).not.toHaveBeenCalled(); + // The mount ping still fires regardless of `isUnlocked` (the + // background is the authoritative gate); only event-driven pings + // are suppressed. + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); addSpy.mockRestore(); }); @@ -117,9 +120,13 @@ describe("useActivityPing", () => { expect(sendMessageToBackground).toHaveBeenCalledTimes(2); }); - it("does not ping on mount when locked", () => { + it("still pings on mount when locked (background is the authoritative gate)", () => { renderHook(() => useActivityPing(false)); - expect(sendMessageToBackground).not.toHaveBeenCalled(); + expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + expect(sendMessageToBackground).toHaveBeenCalledWith({ + type: SERVICE_TYPES.USER_ACTIVITY, + activePublicKey: "", + }); }); }); diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index d8432c0555..1395c5dcc9 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -17,6 +17,19 @@ const ACTIVITY_EVENTS = [ "wheel", ] as const; +const sendPing = () => { + void sendMessageToBackground({ + type: SERVICE_TYPES.USER_ACTIVITY, + // `USER_ACTIVITY` is account-agnostic — the background only cares + // that *some* extension page is active, not which key is selected. + // Send an empty string (rather than null) to match `BaseMessage`'s + // `activePublicKey: string` typing; the background's mismatch + // check (`if (request.activePublicKey && …)`) treats empty-string + // as "skip the check", same as the previous null. + activePublicKey: "", + }); +}; + /** * Ping the background with USER_ACTIVITY messages whenever the user * interacts with the extension page, while the wallet is unlocked. @@ -24,43 +37,38 @@ const ACTIVITY_EVENTS = [ * event in each `PING_THROTTLE_MS` window — accurate to ~8 % on the * 1-minute preset and effectively noise at higher presets. * - * Also pings once on mount, so opening a new Freighter surface (popup, - * sidebar, fullscreen) while the wallet is unlocked itself counts as - * activity. This prevents the wallet from auto-locking mid-flow when a - * dApp triggers a new popup near the end of the idle window. + * Also pings once on mount of every Freighter surface (popup, sidebar, + * fullscreen), unconditionally. Opening a Freighter UI is itself a + * user-initiated action, and the background-side `userActivity` + * handler is the authoritative gate that decides whether the wallet + * is actually unlocked before rearming the alarm — so the popup + * doesn't need to second-guess its own redux state, which can lag + * behind the background (e.g. before `loadAccount` has hydrated + * `hasPrivateKey`). Without firing on mount, a user whose dApp opens + * a new popup mid-session can be locked out mid-flow because the new + * surface's redux store hasn't caught up to the background's session + * state in time to satisfy the previous `isUnlocked` gate. */ export const useActivityPing = (isUnlocked: boolean) => { + // Mount ping: fire-and-forget, exactly once per surface mount, + // regardless of the popup-side `isUnlocked` signal. The background + // gates whether to actually rearm the alarm on the authoritative + // session state. useEffect(() => { - if (!isUnlocked) return undefined; + sendPing(); + }, []); - let lastPingAt = 0; - const ping = () => { - lastPingAt = Date.now(); - void sendMessageToBackground({ - type: SERVICE_TYPES.USER_ACTIVITY, - // `USER_ACTIVITY` is account-agnostic — the background only - // cares that *some* extension page is active, not which key is - // selected. Send an empty string (rather than null) to match - // `BaseMessage`'s `activePublicKey: string` typing; the - // background's mismatch check (`if (request.activePublicKey - // && …)`) treats empty-string as "skip the check", same as - // the previous null. - activePublicKey: "", - }); - }; - - // Ping once on mount so opening a fresh Freighter surface (popup, - // sidebar, fullscreen) while the wallet is unlocked counts as - // activity and rearms the idle alarm. Without this, a user who - // triggers a dApp flow that opens a new popup near the end of the - // idle window can be locked out mid-flow before they ever interact - // with the new surface. - ping(); + // Event-driven pings: only while unlocked. No-op when locked so + // stray scrolls/clicks on the unlock screen don't generate traffic. + useEffect(() => { + if (!isUnlocked) return undefined; + let lastPingAt = Date.now(); const handler = () => { const now = Date.now(); if (now - lastPingAt < PING_THROTTLE_MS) return; - ping(); + lastPingAt = now; + sendPing(); }; for (const evt of ACTIVITY_EVENTS) { From 69ab9588dba55cb4d2c21be978f20e6329e2af37 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 02:27:20 -0700 Subject: [PATCH 13/21] TEMP: instrument auto-lock path for diagnosis (will be reverted) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds console.warn at four points so we can pinpoint which link in the chain is failing: 1. popup useActivityPing.sendPing — fires when the popup attempts to send USER_ACTIVITY (mount or event-driven). 2. popup useActivityPing.sendPing response/error — fires when the background response (or rejection) lands back in the popup. 3. background userActivity handler — fires when BG receives the ping, with hot/HW unlock state. 4. background SessionTimer.resetSession — fires when the alarm is about to be rearmed, with the resolved delay. 5. background initAlarmListener onAlarm — fires when the alarm actually fires (i.e. wallet about to lock). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/background/helpers/session.ts | 5 +++++ extension/src/background/index.ts | 2 ++ .../messageListener/handlers/userActivity.ts | 8 ++++++++ .../src/popup/helpers/hooks/useActivityPing.ts | 14 ++++++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/extension/src/background/helpers/session.ts b/extension/src/background/helpers/session.ts index 90ad68bc0e..f4e7991aac 100644 --- a/extension/src/background/helpers/session.ts +++ b/extension/src/background/helpers/session.ts @@ -50,6 +50,11 @@ export class SessionTimer { */ async resetSession() { const delayInMinutes = await this.getTimeoutMinutes(); + // eslint-disable-next-line no-console + console.warn("[auto-lock] SessionTimer.resetSession", { + ts: Date.now(), + delayInMinutes, + }); await browser?.alarms.create(SESSION_ALARM_NAME, { delayInMinutes }); } diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 0709dc4cfe..ed03b8b633 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -227,6 +227,8 @@ export const initSidebarBehavior = async () => { export const initAlarmListener = () => { browser?.alarms?.onAlarm.addListener(async ({ name }: { name: string }) => { + // eslint-disable-next-line no-console + console.warn("[auto-lock] alarm fired", { ts: Date.now(), name }); const sessionStore = await buildStore(); const localStore = dataStorageAccess(browserLocalStorage); diff --git a/extension/src/background/messageListener/handlers/userActivity.ts b/extension/src/background/messageListener/handlers/userActivity.ts index 358730ecec..731481b838 100644 --- a/extension/src/background/messageListener/handlers/userActivity.ts +++ b/extension/src/background/messageListener/handlers/userActivity.ts @@ -44,6 +44,14 @@ export const userActivity = async ({ const isHwActive = await getIsHardwareWalletActive({ localStore }); const hwUnlocked = isHwActive && !isHardwareWalletLockedSelector(state); + // eslint-disable-next-line no-console + console.warn("[auto-lock] BG userActivity received", { + ts: Date.now(), + hotUnlocked, + hwUnlocked, + isHwActive, + }); + if (!hotUnlocked && !hwUnlocked) { return { ok: false }; } diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index 1395c5dcc9..aa8eeead65 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -18,7 +18,9 @@ const ACTIVITY_EVENTS = [ ] as const; const sendPing = () => { - void sendMessageToBackground({ + // eslint-disable-next-line no-console + console.warn("[auto-lock] popup sendPing called", { ts: Date.now() }); + sendMessageToBackground({ type: SERVICE_TYPES.USER_ACTIVITY, // `USER_ACTIVITY` is account-agnostic — the background only cares // that *some* extension page is active, not which key is selected. @@ -27,7 +29,15 @@ const sendPing = () => { // check (`if (request.activePublicKey && …)`) treats empty-string // as "skip the check", same as the previous null. activePublicKey: "", - }); + }) + .then((res) => { + // eslint-disable-next-line no-console + console.warn("[auto-lock] popup sendPing response", res); + }) + .catch((e) => { + // eslint-disable-next-line no-console + console.warn("[auto-lock] popup sendPing error", e); + }); }; /** From c0f2e1636095b8c0d588b60c63db32f6d91b1a77 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 03:54:42 -0700 Subject: [PATCH 14/21] Accept extension-origin tab senders in popupMessageListener gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the auto-lock-doesn't-reset-on-new-popup bug found via runtime instrumentation: the popup's USER_ACTIVITY mount ping was being rejected by popupMessageListener with { error: 'Unauthorized' }. The isFromExtensionPage check, introduced with sidebar support, was: const isFromExtensionPage = The intent was to reject content scripts (dApp pages) which always carry a sender.tab. The flaw: dApp-spawned signing popups created via browser.windows.create({ type: 'popup', url: 'index.html#...' }) are also full tabs and therefore also have sender.tab set. Their USER_ACTIVITY pings (and REJECT_SIGNING_REQUEST, OPEN_SIDEBAR) were being dropped as Unauthorized — so the new popup never reset the idle alarm, and the user got auto-locked partway through signing, exactly as reported. The reliable distinguishing signal is sender.url: extension pages — whether tabless (browser-action popup, sidepanel) or tabbed (popup window, options page, fullscreen) — have a chrome-extension:// URL, while content scripts have the dApp's page URL. The new check: - sender.id matches our own extension (same as before), AND - either sender.tab is absent (legacy fast path for popup/sidepanel) OR sender.url starts with our extension origin (covers all tabbed extension pages). Adds a regression test in sidebar.test.ts for the windows.create popup case to lock in the fix. The other isFromExtensionPage call sites (REJECT_SIGNING_REQUEST, OPEN_SIDEBAR) had the same latent issue and are fixed by the same change. Reverts the earlier diagnostic console.warn instrumentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/background/helpers/session.ts | 5 --- extension/src/background/index.ts | 2 -- .../messageListener/__tests__/sidebar.test.ts | 35 +++++++++++++++++++ .../messageListener/handlers/userActivity.ts | 8 ----- .../messageListener/popupMessageListener.ts | 22 +++++++++--- .../popup/helpers/hooks/useActivityPing.ts | 14 ++------ 6 files changed, 54 insertions(+), 32 deletions(-) diff --git a/extension/src/background/helpers/session.ts b/extension/src/background/helpers/session.ts index f4e7991aac..90ad68bc0e 100644 --- a/extension/src/background/helpers/session.ts +++ b/extension/src/background/helpers/session.ts @@ -50,11 +50,6 @@ export class SessionTimer { */ async resetSession() { const delayInMinutes = await this.getTimeoutMinutes(); - // eslint-disable-next-line no-console - console.warn("[auto-lock] SessionTimer.resetSession", { - ts: Date.now(), - delayInMinutes, - }); await browser?.alarms.create(SESSION_ALARM_NAME, { delayInMinutes }); } diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index ed03b8b633..0709dc4cfe 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -227,8 +227,6 @@ export const initSidebarBehavior = async () => { export const initAlarmListener = () => { browser?.alarms?.onAlarm.addListener(async ({ name }: { name: string }) => { - // eslint-disable-next-line no-console - console.warn("[auto-lock] alarm fired", { ts: Date.now(), name }); const sessionStore = await buildStore(); const localStore = dataStorageAccess(browserLocalStorage); diff --git a/extension/src/background/messageListener/__tests__/sidebar.test.ts b/extension/src/background/messageListener/__tests__/sidebar.test.ts index 815939d7db..9dd774705f 100644 --- a/extension/src/background/messageListener/__tests__/sidebar.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebar.test.ts @@ -18,6 +18,19 @@ const mockOpen = jest.fn().mockResolvedValue(undefined); runtime: { getURL: (path: string) => `chrome-extension://fake-id${path}` }, }; +// `webextension-polyfill` is mocked to undefined globally; provide a +// minimal shim here so the `isFromExtensionPage` check can compute +// `browser.runtime.getURL("")` and recognize extension-origin URLs. +jest.mock("webextension-polyfill", () => ({ + __esModule: true, + default: { + runtime: { + id: "fake-id", + getURL: (path: string) => `chrome-extension://fake-id${path}`, + }, + }, +})); + const mockSessionStore = { getState: jest.fn().mockReturnValue({ session: { publicKey: "" } }), } as any; @@ -74,6 +87,28 @@ describe("sidebar message handlers", () => { expect(mockOpen).toHaveBeenCalledWith({ windowId: 42 }); }); + it("opens the sidebar when sender is an extension-origin tab (e.g. windows.create popup)", async () => { + // dApp-spawned signing popups created via browser.windows.create + // are full tabs and DO have sender.tab — but their sender.url is + // on the extension origin, distinguishing them from content scripts. + const extensionTabSender = { + tab: { id: 5 }, + url: "chrome-extension://fake-id/index.html#/sign-transaction?foo=bar", + id: "fake-id", + }; + const result = await popupMessageListener( + request as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + extensionTabSender, + ); + expect(result).toEqual({}); + expect(mockSetOptions).toHaveBeenCalled(); + expect(mockOpen).toHaveBeenCalled(); + }); + it("opens the sidebar when sender is from this extension", async () => { const result = await popupMessageListener( request as any, diff --git a/extension/src/background/messageListener/handlers/userActivity.ts b/extension/src/background/messageListener/handlers/userActivity.ts index 731481b838..358730ecec 100644 --- a/extension/src/background/messageListener/handlers/userActivity.ts +++ b/extension/src/background/messageListener/handlers/userActivity.ts @@ -44,14 +44,6 @@ export const userActivity = async ({ const isHwActive = await getIsHardwareWalletActive({ localStore }); const hwUnlocked = isHwActive && !isHardwareWalletLockedSelector(state); - // eslint-disable-next-line no-console - console.warn("[auto-lock] BG userActivity received", { - ts: Date.now(), - hotUnlocked, - hwUnlocked, - isHwActive, - }); - if (!hotUnlocked && !hwUnlocked) { return { ok: false }; } diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index e3dc8c9dcb..f148787e90 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -143,16 +143,28 @@ export const popupMessageListener = ( localStore: DataStorageAccess, keyManager: KeyManager, sessionTimer: SessionTimer, - sender: { tab?: unknown; id?: string }, + sender: { tab?: unknown; id?: string; url?: string }, ) => { const currentState = sessionStore.getState(); const publicKey = publicKeySelector(currentState); - // Content scripts (dapp pages) always carry sender.tab; extension pages do not. - // Also verify the message originates from this extension (sender.id matches), - // guarding against other extensions calling popupMessageListener handlers. + // Content scripts (dapp pages) carry `sender.tab` with a non-extension + // `sender.url`. Extension pages may also carry `sender.tab` when they + // live in their own tab/window (e.g. dApp-spawned signing popups + // created via `browser.windows.create`, fullscreen mode); those still + // have an extension-origin `sender.url`. So the right check is: + // `sender.id` matches our extension AND either there is no tab + // (browser-action popup, sidepanel) OR the URL is on our extension + // origin (popup window, options page, fullscreen). + const extensionOrigin = browser?.runtime?.getURL?.("") ?? ""; + const isFromOwnExtension = + !sender.id || sender.id === browser?.runtime?.id; + const isExtensionUrl = + !!extensionOrigin && + typeof sender.url === "string" && + sender.url.startsWith(extensionOrigin); const isFromExtensionPage = - !sender.tab && (!sender.id || sender.id === browser?.runtime?.id); + isFromOwnExtension && (!sender.tab || isExtensionUrl); if ( request.activePublicKey && diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index aa8eeead65..1395c5dcc9 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -18,9 +18,7 @@ const ACTIVITY_EVENTS = [ ] as const; const sendPing = () => { - // eslint-disable-next-line no-console - console.warn("[auto-lock] popup sendPing called", { ts: Date.now() }); - sendMessageToBackground({ + void sendMessageToBackground({ type: SERVICE_TYPES.USER_ACTIVITY, // `USER_ACTIVITY` is account-agnostic — the background only cares // that *some* extension page is active, not which key is selected. @@ -29,15 +27,7 @@ const sendPing = () => { // check (`if (request.activePublicKey && …)`) treats empty-string // as "skip the check", same as the previous null. activePublicKey: "", - }) - .then((res) => { - // eslint-disable-next-line no-console - console.warn("[auto-lock] popup sendPing response", res); - }) - .catch((e) => { - // eslint-disable-next-line no-console - console.warn("[auto-lock] popup sendPing error", e); - }); + }); }; /** From 7b3e4dd8aa1dc2e42eff13d62ccd61a08a0e0410 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 05:28:57 -0700 Subject: [PATCH 15/21] Drop surface-mount USER_ACTIVITY ping; only user input resets the idle timer Previously, mounting any Freighter surface (popup, sidebar, fullscreen, dApp-spawned signing popup) fired a USER_ACTIVITY ping that reset the idle auto-lock alarm. The intent was to avoid mid-flow lockout when a dApp opens a fresh popup near the end of the idle window. The tradeoff: dApp-triggered signing popups are programmatic. A malicious or compromised dApp could call requestSign() on a timer to spawn fresh popups indefinitely, each one resetting the idle alarm without any actual user presence. This effectively neutralizes the user's auto-lock setting. Align with MetaMask and Phantom: only direct user input inside the Freighter UI (mousedown, keydown, touchstart, wheel) resets the timer. Surface mounts no longer ping. A surface that opens within the idle window still has whatever time was left on the clock; if the user engages with it, the throttled event handler rearms the alarm as expected. The popupMessageListener gate widening from c0f2e163 is retained: it benefits OPEN_SIDEBAR, REJECT_SIGNING_REQUEST, and event-driven USER_ACTIVITY pings sent from extension-origin tabbed surfaces (popup windows, fullscreen, options). The background-side authoritative unlock gate from b3868f9c is also retained as cheap defense-in-depth against stale popup-side redux state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../hooks/__tests__/useActivityPing.test.ts | 35 +++---------------- .../popup/helpers/hooks/useActivityPing.ts | 28 +++++---------- 2 files changed, 13 insertions(+), 50 deletions(-) diff --git a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts index 0ea2e627db..6a6f5089e7 100644 --- a/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts +++ b/extension/src/popup/helpers/hooks/__tests__/useActivityPing.test.ts @@ -88,45 +88,20 @@ describe("useActivityPing", () => { expect.anything(), ); } - // The mount ping still fires regardless of `isUnlocked` (the - // background is the authoritative gate); only event-driven pings - // are suppressed. - expect(sendMessageToBackground).toHaveBeenCalledTimes(1); + expect(sendMessageToBackground).not.toHaveBeenCalled(); addSpy.mockRestore(); }); - it("sends a USER_ACTIVITY ping on mount when unlocked", () => { + it("does not ping on mount (only user input resets the timer)", () => { renderHook(() => useActivityPing(true)); - expect(sendMessageToBackground).toHaveBeenCalledTimes(1); - expect(sendMessageToBackground).toHaveBeenCalledWith({ - type: SERVICE_TYPES.USER_ACTIVITY, - activePublicKey: "", - }); + expect(sendMessageToBackground).not.toHaveBeenCalled(); }); - it("throttles event-driven pings against the mount ping", () => { - renderHook(() => useActivityPing(true)); - - // Mount ping has already fired at t=10_000; event within the - // throttle window is suppressed. - window.dispatchEvent(new MouseEvent("mousedown")); - expect(sendMessageToBackground).toHaveBeenCalledTimes(1); - - // Past the throttle window, events resume firing. - jest.setSystemTime(15_000); - window.dispatchEvent(new MouseEvent("mousedown")); - expect(sendMessageToBackground).toHaveBeenCalledTimes(2); - }); - - it("still pings on mount when locked (background is the authoritative gate)", () => { + it("does not ping on mount when locked", () => { renderHook(() => useActivityPing(false)); - expect(sendMessageToBackground).toHaveBeenCalledTimes(1); - expect(sendMessageToBackground).toHaveBeenCalledWith({ - type: SERVICE_TYPES.USER_ACTIVITY, - activePublicKey: "", - }); + expect(sendMessageToBackground).not.toHaveBeenCalled(); }); }); diff --git a/extension/src/popup/helpers/hooks/useActivityPing.ts b/extension/src/popup/helpers/hooks/useActivityPing.ts index 1395c5dcc9..ee872f17aa 100644 --- a/extension/src/popup/helpers/hooks/useActivityPing.ts +++ b/extension/src/popup/helpers/hooks/useActivityPing.ts @@ -37,33 +37,21 @@ const sendPing = () => { * event in each `PING_THROTTLE_MS` window — accurate to ~8 % on the * 1-minute preset and effectively noise at higher presets. * - * Also pings once on mount of every Freighter surface (popup, sidebar, - * fullscreen), unconditionally. Opening a Freighter UI is itself a - * user-initiated action, and the background-side `userActivity` - * handler is the authoritative gate that decides whether the wallet - * is actually unlocked before rearming the alarm — so the popup - * doesn't need to second-guess its own redux state, which can lag - * behind the background (e.g. before `loadAccount` has hydrated - * `hasPrivateKey`). Without firing on mount, a user whose dApp opens - * a new popup mid-session can be locked out mid-flow because the new - * surface's redux store hasn't caught up to the background's session - * state in time to satisfy the previous `isUnlocked` gate. + * Deliberately does NOT ping on mount. A surface mount is not by + * itself proof of user presence — dApp-spawned signing popups, for + * example, can be triggered programmatically while the user is away. + * Pinging on mount would let a malicious dApp prolong the session + * indefinitely by spamming sign requests. This matches MetaMask and + * Phantom: only direct user input inside the wallet UI resets the + * idle timer. */ export const useActivityPing = (isUnlocked: boolean) => { - // Mount ping: fire-and-forget, exactly once per surface mount, - // regardless of the popup-side `isUnlocked` signal. The background - // gates whether to actually rearm the alarm on the authoritative - // session state. - useEffect(() => { - sendPing(); - }, []); - // Event-driven pings: only while unlocked. No-op when locked so // stray scrolls/clicks on the unlock screen don't generate traffic. useEffect(() => { if (!isUnlocked) return undefined; - let lastPingAt = Date.now(); + let lastPingAt = 0; const handler = () => { const now = Date.now(); if (now - lastPingAt < PING_THROTTLE_MS) return; From 7793c2840d0ff57f69f1e4abdabc81284ad611a7 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 07:52:49 -0700 Subject: [PATCH 16/21] Address review feedback: scope-drift cleanup and small hardening Addresses the consolidated review asks from PR #2802 rounds 1-3, all of which are local cleanups; no externally-observable behavior change beyond the two flagged hardening fixes (saveSettings validation drop and handleSignedHwPayload rearm narrowing). userActivity.ts handler: Rewrite the doc comment to match the shipped behavior. The previous text still described an unconditional surface-mount ping that was removed in 7b3e4dd8 for the security reason captured in that commit message (dApp-spawned signing popups would otherwise extend the alarm without user presence). saveSettings.ts handler: Drop the {error: 'Invalid autoLockTimeoutMinutes'} early-return. The response type has no error variant the popup could surface, and the only producer is a ` only emits values from + // `VALID_AUTO_LOCK_TIMEOUT_MINUTES`, but a malformed message (e.g. + // from a future client revision) should still produce a sensible + // stored value rather than silently dropping the whole save. const localStore = makeLocalStore(); const result = await saveSettings({ request: { ...baseRequest, autoLockTimeoutMinutes: 7 } as any, @@ -220,18 +208,26 @@ describe("saveSettings autoLockTimeoutMinutes", () => { sessionStore: makeSessionStore(), sessionTimer: makeSessionTimer(), }); - expect((result as any).error).toBe("Invalid autoLockTimeoutMinutes"); - expect(localStore.setItem).not.toHaveBeenCalled(); + expect((result as any).error).toBeUndefined(); + expect(localStore.setItem).toHaveBeenCalledWith( + AUTO_LOCK_TIMEOUT_MINUTES_ID, + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + ); }); - it("rejects non-numeric autoLockTimeoutMinutes", async () => { + it("coerces non-numeric autoLockTimeoutMinutes to the default", async () => { + const localStore = makeLocalStore(); const result = await saveSettings({ request: { ...baseRequest, autoLockTimeoutMinutes: "15" } as any, - localStore: makeLocalStore(), + localStore, sessionStore: makeSessionStore(), sessionTimer: makeSessionTimer(), }); - expect((result as any).error).toBe("Invalid autoLockTimeoutMinutes"); + expect((result as any).error).toBeUndefined(); + expect(localStore.setItem).toHaveBeenCalledWith( + AUTO_LOCK_TIMEOUT_MINUTES_ID, + DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, + ); }); it("persists a valid timeout and reschedules when unlocked", async () => { @@ -251,7 +247,6 @@ describe("saveSettings autoLockTimeoutMinutes", () => { ); expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); expect((result as any).autoLockTimeoutMinutes).toBe(30); - expect((result as any).wasLocked).toBeUndefined(); }); it("does not reschedule the timer when the wallet is locked", async () => { @@ -266,19 +261,13 @@ describe("saveSettings autoLockTimeoutMinutes", () => { expect(sessionTimer.stopSession).not.toHaveBeenCalled(); }); - it("rearms (rather than locking) when the user shortens the timeout", async () => { - // Saving settings is itself a user action, so shortening the - // timeout should restart the idle clock with the new value rather - // than synthesizing an immediate lock — even when the new threshold - // is already smaller than the elapsed idle time of the in-flight - // alarm. Set up `alarmsGet` to return an alarm whose remaining time - // (1 min) is much less than the new 5 min timeout, i.e. elapsed - // idle (59 min) ≫ new timeout (5 min). The previous implementation - // would have detected this and locked immediately; the current - // implementation must simply rearm. - alarmsGet.mockResolvedValue({ - scheduledTime: Date.now() + 1 * 60_000, - }); + it("treats a shortened timeout as user activity and rearms (does not lock immediately)", async () => { + // Saving settings is itself a user action: the handler always + // rearms the idle timer with the new timeout when the wallet is + // unlocked. There is no immediate-lock branch even when the new + // threshold is far below the elapsed idle time — the popup never + // sees a `wasLocked` flag and the session store is never mutated + // from this path. const localStore = makeLocalStore(60); const sessionTimer = makeSessionTimer(); const sessionStore = { @@ -286,7 +275,7 @@ describe("saveSettings autoLockTimeoutMinutes", () => { dispatch: jest.fn(), } as any; - const result = await saveSettings({ + await saveSettings({ request: { ...baseRequest, autoLockTimeoutMinutes: 5 } as any, localStore, sessionStore, @@ -296,7 +285,6 @@ describe("saveSettings autoLockTimeoutMinutes", () => { expect(sessionTimer.resetSession).toHaveBeenCalledTimes(1); expect(sessionTimer.stopSession).not.toHaveBeenCalled(); expect(sessionStore.dispatch).not.toHaveBeenCalled(); - expect((result as any).wasLocked).toBeUndefined(); }); }); diff --git a/extension/src/background/messageListener/__tests__/signOut.test.ts b/extension/src/background/messageListener/__tests__/signOut.test.ts index 7c1c85f61d..05287dce29 100644 --- a/extension/src/background/messageListener/__tests__/signOut.test.ts +++ b/extension/src/background/messageListener/__tests__/signOut.test.ts @@ -19,7 +19,7 @@ describe("signOut handler", () => { jest.clearAllMocks(); }); - it("flushes the session store before clearing temporary storage and broadcasting the lock", async () => { + it("stops the alarm before mutating session state and broadcasts the lock", async () => { const localStore = { getItem: jest.fn().mockResolvedValue("MNEMONIC_PHRASE_CONFIRMED"), remove: jest.fn().mockResolvedValue(undefined), @@ -36,16 +36,27 @@ describe("signOut handler", () => { await signOut({ localStore, sessionStore, sessionTimer }); + expect(sessionTimer.stopSession).toHaveBeenCalledTimes(1); expect(mockFlushSessionStore).toHaveBeenCalledWith(sessionStore); expect(localStore.remove).toHaveBeenCalledTimes(1); expect(mockBroadcastSessionState).toHaveBeenCalledWith( SERVICE_TYPES.SESSION_LOCKED, ); + + // stopSession must run before any state mutation so a pending + // alarm can't fire `clearSession` (and emit a duplicate + // SESSION_LOCKED broadcast) between the dispatch and the clear. + expect( + sessionTimer.stopSession.mock.invocationCallOrder[0], + ).toBeLessThan(sessionStore.dispatch.mock.invocationCallOrder[0]); + expect( + sessionStore.dispatch.mock.invocationCallOrder[0], + ).toBeLessThan(mockFlushSessionStore.mock.invocationCallOrder[0]); expect( mockFlushSessionStore.mock.invocationCallOrder[0], ).toBeLessThan(localStore.remove.mock.invocationCallOrder[0]); expect( - mockBroadcastSessionState.mock.invocationCallOrder[0], - ).toBeGreaterThan(mockFlushSessionStore.mock.invocationCallOrder[0]); + localStore.remove.mock.invocationCallOrder[0], + ).toBeLessThan(mockBroadcastSessionState.mock.invocationCallOrder[0]); }); }); diff --git a/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts b/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts index 32acc81b6a..a950d82f85 100644 --- a/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts +++ b/extension/src/background/messageListener/handlers/handleSignedHwPayload.ts @@ -18,13 +18,6 @@ export const handleSignedHwPayload = async ({ }) => { const { signedPayload, uuid } = request; - // A user just completed a hardware-wallet signature — that is a real - // user action, so extend the idle session. Without this the popup - // ping is the only path that refreshes the alarm, and a slow HW - // signing flow could outlast the timeout while the user is actively - // working. - await sessionTimer.resetSession(); - if (!uuid) { captureException("handleSignedHwPayload: missing uuid in request"); return { error: "Transaction not found" }; @@ -40,6 +33,12 @@ export const handleSignedHwPayload = async ({ transactionResponse && typeof transactionResponse.response === "function" ) { + // A user just completed a hardware-wallet signature — that is a + // real user action, so extend the idle session. We only rearm on + // the success branch; a malformed request or a missing queue + // entry is never a legitimate signal of user presence and must + // not be allowed to extend the deadline. + await sessionTimer.resetSession(); transactionResponse.response(signedPayload); return {}; } diff --git a/extension/src/background/messageListener/handlers/saveSettings.ts b/extension/src/background/messageListener/handlers/saveSettings.ts index 5638b3c59b..192e3df532 100644 --- a/extension/src/background/messageListener/handlers/saveSettings.ts +++ b/extension/src/background/messageListener/handlers/saveSettings.ts @@ -1,10 +1,7 @@ import { Store } from "redux"; import { SaveSettingsMessage } from "@shared/api/types/message-request"; -import { - coerceAutoLockTimeoutMinutes, - isValidAutoLockTimeoutMinutes, -} from "@shared/constants/autoLock"; +import { coerceAutoLockTimeoutMinutes } from "@shared/constants/autoLock"; import { getAllowList, getFeatureFlags, @@ -47,9 +44,16 @@ export const saveSettings = async ({ autoLockTimeoutMinutes, } = request; - if (!isValidAutoLockTimeoutMinutes(autoLockTimeoutMinutes)) { - return { error: "Invalid autoLockTimeoutMinutes" }; - } + // `autoLockTimeoutMinutes` originates from the Preferences `` deliver this as a string at runtime. Keep the - // field type honest and convert it explicitly in `handleSubmit`. - autoLockTimeoutMinutesValue: string; } const handleSubmit = async (formValue: SettingValues) => { @@ -72,7 +42,6 @@ export const Preferences = () => { isDataSharingAllowedValue, isHideDustEnabledValue, isOpenSidebarByDefaultValue, - autoLockTimeoutMinutesValue, } = formValue; await dispatch( @@ -81,9 +50,7 @@ export const Preferences = () => { isDataSharingAllowed: isDataSharingAllowedValue, isHideDustEnabled: isHideDustEnabledValue, isOpenSidebarByDefault: isOpenSidebarByDefaultValue, - autoLockTimeoutMinutes: coerceAutoLockTimeoutMinutes( - Number(autoLockTimeoutMinutesValue), - ), + autoLockTimeoutMinutes, }), ); }; @@ -141,7 +108,6 @@ export const Preferences = () => { isDataSharingAllowed, isHideDustEnabled, isOpenSidebarByDefault, - autoLockTimeoutMinutes, } = state.data.settings; const initialValues: SettingValues = { @@ -149,11 +115,6 @@ export const Preferences = () => { isDataSharingAllowedValue: isDataSharingAllowed, isHideDustEnabledValue: isHideDustEnabled, isOpenSidebarByDefaultValue: isOpenSidebarByDefault ?? false, - autoLockTimeoutMinutesValue: String( - coerceAutoLockTimeoutMinutes( - autoLockTimeoutMinutes ?? DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES, - ), - ), }; return ( @@ -246,34 +207,6 @@ export const Preferences = () => { )} - -
-
- {t("Auto-lock after")} -
- - {VALID_AUTO_LOCK_TIMEOUT_MINUTES.map((minutes) => ( - - ))} - -
-
- - {t( - "Lock your wallet after this much time without interaction", - )} - -
diff --git a/extension/src/popup/views/Security/index.tsx b/extension/src/popup/views/Security/index.tsx index 12b5f4f70a..ea572163b7 100644 --- a/extension/src/popup/views/Security/index.tsx +++ b/extension/src/popup/views/Security/index.tsx @@ -52,6 +52,12 @@ export const Security = () => { > {t("Advanced settings")} + } + > + {t("Auto-lock timer")} + {/* { openTab(newTabHref(ROUTES.accountMigration)); From 191f6cf6f387141485c72b7fb9ded331661aa478 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 29 May 2026 13:12:48 -0700 Subject: [PATCH 20/21] Change 4h option to 6h, add 12h option, default to 12h - Replace 240 min (4h) with 360 min (6h) in VALID_AUTO_LOCK_TIMEOUT_MINUTES - Add 720 min (12h) to the list - Change DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES from 15 to 720 (12h) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/constants/autoLock.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/@shared/constants/autoLock.ts b/@shared/constants/autoLock.ts index 41175e92e5..f27e8b529e 100644 --- a/@shared/constants/autoLock.ts +++ b/@shared/constants/autoLock.ts @@ -7,13 +7,13 @@ * an extension page resets the timer. */ export const VALID_AUTO_LOCK_TIMEOUT_MINUTES = [ - 1, 5, 15, 30, 60, 240, 1440, + 1, 5, 15, 30, 60, 360, 720, 1440, ] as const; export type AutoLockTimeoutMinutes = (typeof VALID_AUTO_LOCK_TIMEOUT_MINUTES)[number]; -export const DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES: AutoLockTimeoutMinutes = 15; +export const DEFAULT_AUTO_LOCK_TIMEOUT_MINUTES: AutoLockTimeoutMinutes = 720; export const isValidAutoLockTimeoutMinutes = ( value: unknown, From bc002332b37809ca726833fb5e314dae004f480b Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 09:09:15 -0700 Subject: [PATCH 21/21] Restore saveSettings error envelope check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background's popupMessageListener returns { error: "..." } when activePublicKey doesn't match (e.g. account switched in another surface). Commit c9487e61 removed the only path that surfaced this envelope, so the error object was silently treated as a SaveSettingsResponse — writing undefined into every redux field and crashing downstream selectors. Restore a runtime check that throws on error envelopes before returning the response. The sendMessageToBackground generic is widened to include the error shape so the 'in' narrowing is type-safe without needing a type assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- @shared/api/internal.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 8274856c50..e2180b6a63 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -1647,7 +1647,9 @@ export const saveSettings = async ({ }; try { - response = await sendMessageToBackground({ + const raw = await sendMessageToBackground< + SaveSettingsResponse | { error: string } + >({ activePublicKey, isDataSharingAllowed, isMemoValidationEnabled, @@ -1656,6 +1658,12 @@ export const saveSettings = async ({ autoLockTimeoutMinutes, type: SERVICE_TYPES.SAVE_SETTINGS, }); + + if ("error" in raw && raw.error) { + throw new Error(raw.error); + } + + response = raw as SaveSettingsResponse; } catch (e) { console.error(e); }