From 466cffeceff02ad117a23b243eabfc680e95f554 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Tue, 10 Nov 2026 15:43:16 -0700 Subject: [PATCH 1/6] fix(accounts): serialize account-mutating background messages Messages that change accounts, sources or session state now run one at a time behind a single lock in the background message listener. Read-only messages skip the lock. --- .../concurrentAccountMutations.test.ts | 322 ++++++++++++++++++ .../__tests__/initAlarmListener.test.ts | 5 +- .../__tests__/account-mutation-lock.test.ts | 48 +++ .../helpers/account-mutation-lock.ts | 52 +++ extension/src/background/index.ts | 118 ++++--- 5 files changed, 503 insertions(+), 42 deletions(-) create mode 100644 extension/src/background/__tests__/concurrentAccountMutations.test.ts create mode 100644 extension/src/background/helpers/__tests__/account-mutation-lock.test.ts create mode 100644 extension/src/background/helpers/account-mutation-lock.ts diff --git a/extension/src/background/__tests__/concurrentAccountMutations.test.ts b/extension/src/background/__tests__/concurrentAccountMutations.test.ts new file mode 100644 index 0000000000..30796894fd --- /dev/null +++ b/extension/src/background/__tests__/concurrentAccountMutations.test.ts @@ -0,0 +1,322 @@ +import { randomUUID } from "crypto"; +import { Keypair } from "stellar-sdk"; + +import { SERVICE_TYPES } from "@shared/constants/services"; +import { + ACCOUNT_SOURCE_VECTORS, + AUTH_KEYPAIR_VECTORS, +} from "@shared/api/helpers/authKeypairVectors"; +import { Account, SecretSource } from "@shared/api/types"; +import { + ACCOUNT_PROVENANCE_ID, + DERIVATION_INDICES_ID, + KEY_ID_LIST, + SECRET_SOURCES_ID, + TEMPORARY_STORE_ID, +} from "constants/localStorageTypes"; + +// #112: account-mutating messages from two surfaces (popup, sidebar, +// fullscreen) must not lose each other's writes. Each message builds its own +// session store from storage.session, so these run through the real +// onMessage listener with real storage.local and storage.session stand-ins. + +const [PHRASE_1, PHRASE_2] = AUTH_KEYPAIR_VECTORS; +const [ROWS_1, ROWS_2] = ACCOUNT_SOURCE_VECTORS.map(({ accounts }) => accounts); +const PASSWORD = "test"; + +const makeArea = () => { + const data: Record = {}; + // Real storage writes cross an IPC boundary; a delay lets a test model a + // write that hasn't landed when the next reader looks. + const latency = { ms: 0 }; + const pick = (keys: unknown) => { + if (keys === null || keys === undefined) return { ...data }; + const list = typeof keys === "string" ? [keys] : (keys as string[]); + return Object.fromEntries( + list.filter((k) => k in data).map((k) => [k, data[k]]), + ); + }; + return { + data, + latency, + get: jest.fn(async (keys: unknown) => structuredClone(pick(keys))), + set: jest.fn(async (items: Record) => { + const copy = structuredClone(items); + if (latency.ms) { + await new Promise((resolve) => setTimeout(resolve, latency.ms)); + } + Object.assign(data, copy); + }), + remove: jest.fn(async (keys: string | string[]) => { + for (const k of Array.isArray(keys) ? keys : [keys]) delete data[k]; + }), + clear: jest.fn(async () => { + for (const k of Object.keys(data)) delete data[k]; + }), + getBytesInUse: jest.fn(async () => 0), + onChanged: { addListener: jest.fn(), removeListener: jest.fn() }, + }; +}; + +const mockLocal = makeArea(); +const mockSession = makeArea(); +let mockMessageListener: (request: unknown, sender: unknown) => Promise; + +jest.mock("webextension-polyfill", () => ({ + __esModule: true, + default: { + storage: { + get local() { + return mockLocal; + }, + get session() { + return mockSession; + }, + }, + runtime: { + id: "fake-extension-id", + getURL: (path: string) => `chrome-extension://fake-extension-id${path}`, + sendMessage: jest.fn().mockResolvedValue(undefined), + onMessage: { + addListener: (fn: typeof mockMessageListener) => { + mockMessageListener = fn; + }, + }, + }, + alarms: { + create: jest.fn().mockResolvedValue(undefined), + clear: jest.fn().mockResolvedValue(true), + onAlarm: { addListener: jest.fn() }, + }, + }, +})); + +jest.mock("background/helpers/link-sources", () => ({ + linkWalletSources: jest.fn().mockResolvedValue(undefined), +})); + +// StellarHDWallet.fromMnemonic cannot run under jest (bip39 CJS interop), so +// derive from the pinned (mnemonic, index) rows. +const mockGenerated: string[] = []; +jest.mock("stellar-hd-wallet", () => { + const { ACCOUNT_SOURCE_VECTORS: vectors } = jest.requireActual( + "@shared/api/helpers/authKeypairVectors", + ); + return { + generateMnemonic: () => mockGenerated.shift(), + fromMnemonic: (mnemonic: string) => { + const rows = vectors.find((v: any) => v.mnemonic === mnemonic).accounts; + return { + getPublicKey: (i: number) => rows[i].publicKey, + getSecret: (i: number) => rows[i].secret, + }; + }, + }; +}); + +if (!global.crypto.randomUUID) { + Object.defineProperty(global.crypto, "randomUUID", { value: randomUUID }); +} + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { initExtensionMessageListener } = require("background/index"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { getAccountNameList } = require("background/helpers/account"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { dataStorageAccess } = require("background/helpers/dataStorageAccess"); + +const send = (request: object) => + mockMessageListener(request, { id: "fake-extension-id" }); + +const session = () => + JSON.parse((mockSession.data["session-store"] as string) ?? "{}").session ?? + {}; + +const allAccounts = (): Account[] => session().allAccounts ?? []; + +const local = (key: string) => mockLocal.data[key] as any; + +const createWallet = async () => { + mockGenerated.push(PHRASE_1.mnemonic); + await send({ + type: SERVICE_TYPES.CREATE_ACCOUNT, + password: PASSWORD, + isOverwritingAccount: false, + }); +}; + +const generateSecondPhrase = async () => { + mockGenerated.push(PHRASE_2.mnemonic); + const { pendingPhraseId } = await send({ + type: SERVICE_TYPES.GENERATE_RECOVERY_PHRASE, + }); + await send({ + type: SERVICE_TYPES.ADD_RECOVERY_PHRASE_SOURCE, + password: PASSWORD, + pendingPhraseId, + }); +}; + +const importKey = (secret: string) => + send({ + type: SERVICE_TYPES.IMPORT_ACCOUNT, + password: PASSWORD, + privateKey: secret, + }); + +const addAccount = (sourceId: string) => + send({ type: SERVICE_TYPES.ADD_ACCOUNT, password: PASSWORD, sourceId }); + +describe("concurrent account-mutating messages", () => { + jest.setTimeout(120_000); + + beforeAll(() => { + initExtensionMessageListener(); + }); + + beforeEach(async () => { + mockSession.latency.ms = 0; + await mockLocal.clear(); + await mockSession.clear(); + mockGenerated.length = 0; + global.fetch = jest.fn(() => + Promise.resolve({ + json: () => Promise.resolve({ account_id: "funded" }), + }), + ) as unknown as typeof fetch; + await createWallet(); + }); + + it("keeps both of two concurrent secret-key imports", async () => { + const [a, b] = [Keypair.random(), Keypair.random()]; + + const results = await Promise.all([ + importKey(a.secret()), + importKey(b.secret()), + ]); + + expect(results.map((r) => r.error)).toEqual([undefined, undefined]); + const imported = allAccounts().filter(({ publicKey }) => + [a.publicKey(), b.publicKey()].includes(publicKey), + ); + expect(imported).toHaveLength(2); + expect(new Set(allAccounts().map(({ name }) => name)).size).toBe(3); + + const keyIds = local(KEY_ID_LIST) as string[]; + expect(keyIds).toHaveLength(3); + expect(Object.keys(local(TEMPORARY_STORE_ID))).toEqual( + expect.arrayContaining(keyIds), + ); + const names = await getAccountNameList({ + localStore: dataStorageAccess(mockLocal), + }); + expect(Object.keys(names).sort()).toEqual([...keyIds].sort()); + expect(new Set(Object.values(names as Record)).size).toBe( + 3, + ); + + const sources = Object.values( + local(SECRET_SOURCES_ID) as Record, + ); + expect(sources).toHaveLength(3); + expect(new Set(sources.map(({ label }) => label)).size).toBe(3); + }); + + it("hands the next holder the previous holder's session when writes are slow", async () => { + mockSession.latency.ms = 50; + const [a, b] = [Keypair.random(), Keypair.random()]; + + await Promise.all([importKey(a.secret()), importKey(b.secret())]); + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(allAccounts().map(({ publicKey }) => publicKey)).toEqual( + expect.arrayContaining([a.publicKey(), b.publicKey()]), + ); + }); + + it("gives two concurrent adds on one phrase distinct indices", async () => { + const results = await Promise.all([ + addAccount(PHRASE_1.userId), + addAccount(PHRASE_1.userId), + ]); + + expect(results.map((r) => r.publicKey).sort()).toEqual( + [ROWS_1[1].publicKey, ROWS_1[2].publicKey].sort(), + ); + expect(local(DERIVATION_INDICES_ID)[PHRASE_1.userId]).toEqual([0, 1, 2]); + expect(allAccounts()).toHaveLength(3); + }); + + it("keeps both phrases' marks for concurrent adds on different phrases", async () => { + await generateSecondPhrase(); + + await Promise.all([ + addAccount(PHRASE_1.userId), + addAccount(PHRASE_2.userId), + ]); + + expect(local(DERIVATION_INDICES_ID)).toEqual({ + [PHRASE_1.userId]: [0, 1], + [PHRASE_2.userId]: [0, 1], + }); + expect(allAccounts().map(({ publicKey }) => publicKey)).toEqual( + expect.arrayContaining([ROWS_1[1].publicKey, ROWS_2[1].publicKey]), + ); + }); + + it("gives a concurrent add and import distinct names and provenance", async () => { + const key = Keypair.random(); + + await Promise.all([addAccount(PHRASE_1.userId), importKey(key.secret())]); + + expect(allAccounts()).toHaveLength(3); + expect(new Set(allAccounts().map(({ name }) => name)).size).toBe(3); + const provenance = local(ACCOUNT_PROVENANCE_ID); + for (const { id } of allAccounts()) { + expect(provenance[id]).toBeDefined(); + } + }); + + it("keeps a rename made while an import is in flight", async () => { + const [first] = allAccounts(); + const key = Keypair.random(); + + await Promise.all([ + importKey(key.secret()), + send({ + type: SERVICE_TYPES.UPDATE_ACCOUNT_NAME, + id: first.id, + accountName: "Renamed", + }), + ]); + + expect(allAccounts().find(({ id }) => id === first.id)?.name).toBe( + "Renamed", + ); + expect( + allAccounts().find(({ publicKey }) => publicKey === key.publicKey()), + ).toBeDefined(); + }); + + it("stays locked when a sign-out lands during an import", async () => { + const key = Keypair.random(); + + const importing = importKey(key.secret()); + const signingOut = send({ type: SERVICE_TYPES.SIGN_OUT }); + await Promise.all([importing, signingOut]); + + expect(session().publicKey).toBeFalsy(); + }); + + it("answers a read-only message without waiting for an in-flight import", async () => { + const order: string[] = []; + const key = Keypair.random(); + + await Promise.all([ + importKey(key.secret()).then(() => order.push("import")), + send({ type: SERVICE_TYPES.LOAD_ACCOUNT }).then(() => order.push("load")), + ]); + + expect(order).toEqual(["load", "import"]); + }); +}); diff --git a/extension/src/background/__tests__/initAlarmListener.test.ts b/extension/src/background/__tests__/initAlarmListener.test.ts index fc11d779c6..7fd17c8991 100644 --- a/extension/src/background/__tests__/initAlarmListener.test.ts +++ b/extension/src/background/__tests__/initAlarmListener.test.ts @@ -32,6 +32,7 @@ jest.mock("background/helpers/session", () => { jest.mock("background/store", () => ({ buildStore: jest.fn().mockResolvedValue({}), + flushSessionStore: jest.fn().mockResolvedValue(undefined), })); jest.mock("background/helpers/dataStorageAccess", () => ({ @@ -67,7 +68,9 @@ describe("initAlarmListener", () => { 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."), + new Error( + "Could not establish connection. Receiving end does not exist.", + ), ); await expect( diff --git a/extension/src/background/helpers/__tests__/account-mutation-lock.test.ts b/extension/src/background/helpers/__tests__/account-mutation-lock.test.ts new file mode 100644 index 0000000000..bfe659370e --- /dev/null +++ b/extension/src/background/helpers/__tests__/account-mutation-lock.test.ts @@ -0,0 +1,48 @@ +import { withAccountMutationLock } from "background/helpers/account-mutation-lock"; + +const deferred = () => { + let resolve!: () => void; + let reject!: (e: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +describe("withAccountMutationLock", () => { + it("starts the next holder only after the previous one settles", async () => { + const first = deferred(); + const events: string[] = []; + + const a = withAccountMutationLock(async () => { + events.push("a:start"); + await first.promise; + events.push("a:end"); + return "a"; + }); + const b = withAccountMutationLock(async () => { + events.push("b:start"); + return "b"; + }); + + await Promise.resolve(); + await Promise.resolve(); + expect(events).toEqual(["a:start"]); + + first.resolve(); + + expect(await Promise.all([a, b])).toEqual(["a", "b"]); + expect(events).toEqual(["a:start", "a:end", "b:start"]); + }); + + it("releases the lock when a holder rejects", async () => { + const failing = withAccountMutationLock(async () => { + throw new Error("boom"); + }); + const next = withAccountMutationLock(async () => "next"); + + await expect(failing).rejects.toThrow("boom"); + await expect(next).resolves.toBe("next"); + }); +}); diff --git a/extension/src/background/helpers/account-mutation-lock.ts b/extension/src/background/helpers/account-mutation-lock.ts new file mode 100644 index 0000000000..49aa8f134b --- /dev/null +++ b/extension/src/background/helpers/account-mutation-lock.ts @@ -0,0 +1,52 @@ +import { SERVICE_TYPES } from "@shared/constants/services"; + +/* + Messages that change the wallet's accounts, sources or session. Each one + builds its own session store from storage.session and flushes the whole + state back on every dispatch, and the account lists in storage.local are + read-modify-writes. Two of these from different surfaces (popup, sidebar, + fullscreen) must not interleave, or the later writer drops the earlier + one's account, name, label, index or lock state. + + Read-only messages stay off the lock so an import's Scrypt doesn't stall + them. +*/ +export const ACCOUNT_MUTATING_SERVICE_TYPES: ReadonlySet = new Set([ + SERVICE_TYPES.CREATE_ACCOUNT, + SERVICE_TYPES.RECOVER_ACCOUNT, + SERVICE_TYPES.ADD_ACCOUNT, + SERVICE_TYPES.IMPORT_ACCOUNT, + SERVICE_TYPES.IMPORT_HARDWARE_WALLET, + SERVICE_TYPES.MIGRATE_ACCOUNTS, + SERVICE_TYPES.GENERATE_RECOVERY_PHRASE, + SERVICE_TYPES.ADD_RECOVERY_PHRASE_SOURCE, + SERVICE_TYPES.CONFIRM_MNEMONIC_PHRASE, + SERVICE_TYPES.CONFIRM_MIGRATED_MNEMONIC_PHRASE, + // Both run loginToAllAccounts, whose unlock pass writes the source records + // and rewrites the temporary store. + SERVICE_TYPES.CONFIRM_PASSWORD, + SERVICE_TYPES.SHOW_BACKUP_PHRASE, + SERVICE_TYPES.UPDATE_ACCOUNT_NAME, + SERVICE_TYPES.MAKE_ACCOUNT_ACTIVE, + SERVICE_TYPES.SIGN_OUT, + SERVICE_TYPES.RESET_WALLET, + // These dispatch to the session store too, so their flush could overwrite + // another holder's session. + SERVICE_TYPES.GET_MIGRATED_MNEMONIC_PHRASE, + SERVICE_TYPES.RESET_EXP_DATA, +]); + +// One chain per background context is enough: MV3 runs a single service +// worker per extension and Firefox a single background page. +let tail: Promise = Promise.resolve(); + +/* + Runs `fn` once every earlier holder has settled. A rejection still releases + the lock. Callers take it before building the session store, so the store + they load already reflects the previous holder's writes. +*/ +export const withAccountMutationLock = (fn: () => Promise) => { + const run = tail.then(fn); + tail = run.catch(() => undefined); + return run; +}; diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 9e20f856fe..170a73183c 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -5,7 +5,7 @@ import { SERVICE_TYPES, } from "@shared/constants/services"; import { ExternalRequest, Response } from "@shared/api/types"; -import { buildStore } from "background/store"; +import { buildStore, flushSessionStore } from "background/store"; import { popupMessageListener, @@ -46,6 +46,10 @@ import { dataStorageAccess, browserLocalStorage, } from "./helpers/dataStorageAccess"; +import { + ACCOUNT_MUTATING_SERVICE_TYPES, + withAccountMutationLock, +} from "./helpers/account-mutation-lock"; import { IS_OPEN_SIDEBAR_BY_DEFAULT_ID } from "constants/localStorageTypes"; import { ServiceMessageRequest } from "@shared/api/types/message-request"; import { @@ -147,49 +151,76 @@ export const initSidebarConnectionListener = () => { export const initExtensionMessageListener = () => { browser?.runtime?.onMessage?.addListener( - async (request: unknown, sender: browser.Runtime.MessageSender) => { - const sessionStore = await buildStore(); - const localStore = dataStorageAccess(browserLocalStorage); - const localKeyStore = new BrowserStorageKeyStore(); - localKeyStore.configure({ - storage: browserLocalStorage as BrowserStorageConfigParams["storage"], - }); - const keyManager = new KeyManager({ - keyStore: localKeyStore, - }); - keyManager.registerEncrypter(ScryptEncrypter); - // todo this is kinda ugly - const req = request as ExternalRequest | Response; - let res; - - if (Object.values(SERVICE_TYPES).includes(req.type as SERVICE_TYPES)) { - res = await popupMessageListener( - req as ServiceMessageRequest, - sessionStore, - localStore, - keyManager, - sessionTimer, - sender, - ); - } + (request: unknown, sender: browser.Runtime.MessageSender) => { + const type = (request as { type?: unknown } | null)?.type; if ( - Object.values(EXTERNAL_SERVICE_TYPES).includes( - req.type as EXTERNAL_SERVICE_TYPES, - ) + typeof type === "string" && + ACCOUNT_MUTATING_SERVICE_TYPES.has(type) ) { - res = await freighterApiMessageListener( - req as ExternalRequest, - sender, - sessionStore, - localStore, + // The lock covers building the store too: a store loaded before the + // previous holder finished would flush its stale state over that + // holder's writes. + return withAccountMutationLock(() => + handleExtensionMessage(request, sender, true), ); } - - return res; + return handleExtensionMessage(request, sender, false); }, ); }; +const handleExtensionMessage = async ( + request: unknown, + sender: browser.Runtime.MessageSender, + flushBeforeRelease: boolean, +) => { + const sessionStore = await buildStore(); + const localStore = dataStorageAccess(browserLocalStorage); + const localKeyStore = new BrowserStorageKeyStore(); + localKeyStore.configure({ + storage: browserLocalStorage as BrowserStorageConfigParams["storage"], + }); + const keyManager = new KeyManager({ + keyStore: localKeyStore, + }); + keyManager.registerEncrypter(ScryptEncrypter); + // todo this is kinda ugly + const req = request as ExternalRequest | Response; + let res; + + if (Object.values(SERVICE_TYPES).includes(req.type as SERVICE_TYPES)) { + res = await popupMessageListener( + req as ServiceMessageRequest, + sessionStore, + localStore, + keyManager, + sessionTimer, + sender, + ); + } + if ( + Object.values(EXTERNAL_SERVICE_TYPES).includes( + req.type as EXTERNAL_SERVICE_TYPES, + ) + ) { + res = await freighterApiMessageListener( + req as ExternalRequest, + sender, + sessionStore, + localStore, + ); + } + + if (flushBeforeRelease) { + // The store's subscribe flush is fire-and-forget. Wait for the final + // state to land before the lock passes to the next holder, which + // loads the store from it. + await flushSessionStore(sessionStore); + } + + return res; +}; + export const initInstalledListener = () => { browser?.runtime?.onInstalled.addListener(async ({ reason, temporary }) => { if (temporary) { @@ -231,12 +262,17 @@ export const initSidebarBehavior = async () => { export const initAlarmListener = () => { browser?.alarms?.onAlarm.addListener(async ({ name }: { name: string }) => { - const sessionStore = await buildStore(); - const localStore = dataStorageAccess(browserLocalStorage); + if (name !== SESSION_ALARM_NAME) { + return; + } - if (name === SESSION_ALARM_NAME) { + // Locking mid-import must not be undone by the import's later flush. + await withAccountMutationLock(async () => { + const sessionStore = await buildStore(); + const localStore = dataStorageAccess(browserLocalStorage); await clearSession({ sessionStore, localStore }); - await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); - } + await flushSessionStore(sessionStore); + }); + await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); }); }; From 14207db0c0bd19017b95128c0db7e8609296f3fb Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 11 Nov 2026 12:51:43 -0700 Subject: [PATCH 2/6] fix(accounts): skip active-key check for account-scoped messages Sign-out, rename, switch, add and import skip the active-key mismatch check, so they still run when queued behind another surface's import or switch. The popup's sign-out now fails when the background refuses it. --- .../concurrentAccountMutations.test.ts | 86 +++++++++++++++---- .../messageListener/popupMessageListener.ts | 22 ++++- .../ducks/__tests__/accountServices.test.ts | 30 +++++++ extension/src/popup/ducks/accountServices.ts | 4 +- 4 files changed, 122 insertions(+), 20 deletions(-) diff --git a/extension/src/background/__tests__/concurrentAccountMutations.test.ts b/extension/src/background/__tests__/concurrentAccountMutations.test.ts index 30796894fd..9f4e776338 100644 --- a/extension/src/background/__tests__/concurrentAccountMutations.test.ts +++ b/extension/src/background/__tests__/concurrentAccountMutations.test.ts @@ -157,15 +157,26 @@ const generateSecondPhrase = async () => { }); }; -const importKey = (secret: string) => +// The popup sends the key it last saw as active. Callers read it before +// starting concurrent sends, so a message queued behind an import or switch +// carries the stale key, as it would from a second surface. +const activeKey = (): string => session().publicKey; + +const importKey = (secret: string, activePublicKey: string) => send({ type: SERVICE_TYPES.IMPORT_ACCOUNT, + activePublicKey, password: PASSWORD, privateKey: secret, }); -const addAccount = (sourceId: string) => - send({ type: SERVICE_TYPES.ADD_ACCOUNT, password: PASSWORD, sourceId }); +const addAccount = (sourceId: string, activePublicKey: string) => + send({ + type: SERVICE_TYPES.ADD_ACCOUNT, + activePublicKey, + password: PASSWORD, + sourceId, + }); describe("concurrent account-mutating messages", () => { jest.setTimeout(120_000); @@ -189,10 +200,11 @@ describe("concurrent account-mutating messages", () => { it("keeps both of two concurrent secret-key imports", async () => { const [a, b] = [Keypair.random(), Keypair.random()]; + const active = activeKey(); const results = await Promise.all([ - importKey(a.secret()), - importKey(b.secret()), + importKey(a.secret(), active), + importKey(b.secret(), active), ]); expect(results.map((r) => r.error)).toEqual([undefined, undefined]); @@ -225,8 +237,12 @@ describe("concurrent account-mutating messages", () => { it("hands the next holder the previous holder's session when writes are slow", async () => { mockSession.latency.ms = 50; const [a, b] = [Keypair.random(), Keypair.random()]; + const active = activeKey(); - await Promise.all([importKey(a.secret()), importKey(b.secret())]); + await Promise.all([ + importKey(a.secret(), active), + importKey(b.secret(), active), + ]); await new Promise((resolve) => setTimeout(resolve, 200)); expect(allAccounts().map(({ publicKey }) => publicKey)).toEqual( @@ -235,9 +251,10 @@ describe("concurrent account-mutating messages", () => { }); it("gives two concurrent adds on one phrase distinct indices", async () => { + const active = activeKey(); const results = await Promise.all([ - addAccount(PHRASE_1.userId), - addAccount(PHRASE_1.userId), + addAccount(PHRASE_1.userId, active), + addAccount(PHRASE_1.userId, active), ]); expect(results.map((r) => r.publicKey).sort()).toEqual( @@ -249,12 +266,15 @@ describe("concurrent account-mutating messages", () => { it("keeps both phrases' marks for concurrent adds on different phrases", async () => { await generateSecondPhrase(); + const active = activeKey(); - await Promise.all([ - addAccount(PHRASE_1.userId), - addAccount(PHRASE_2.userId), + const results = await Promise.all([ + addAccount(PHRASE_1.userId, active), + addAccount(PHRASE_2.userId, active), ]); + expect(results.map((r) => r.error)).toEqual([undefined, undefined]); + expect(local(DERIVATION_INDICES_ID)).toEqual({ [PHRASE_1.userId]: [0, 1], [PHRASE_2.userId]: [0, 1], @@ -266,8 +286,14 @@ describe("concurrent account-mutating messages", () => { it("gives a concurrent add and import distinct names and provenance", async () => { const key = Keypair.random(); + const active = activeKey(); + + const results = await Promise.all([ + addAccount(PHRASE_1.userId, active), + importKey(key.secret(), active), + ]); - await Promise.all([addAccount(PHRASE_1.userId), importKey(key.secret())]); + expect(results.map((r) => r.error)).toEqual([undefined, undefined]); expect(allAccounts()).toHaveLength(3); expect(new Set(allAccounts().map(({ name }) => name)).size).toBe(3); @@ -280,16 +306,20 @@ describe("concurrent account-mutating messages", () => { it("keeps a rename made while an import is in flight", async () => { const [first] = allAccounts(); const key = Keypair.random(); + const active = activeKey(); - await Promise.all([ - importKey(key.secret()), + const results = await Promise.all([ + importKey(key.secret(), active), send({ type: SERVICE_TYPES.UPDATE_ACCOUNT_NAME, + activePublicKey: active, id: first.id, accountName: "Renamed", }), ]); + expect(results.map((r) => r.error)).toEqual([undefined, undefined]); + expect(allAccounts().find(({ id }) => id === first.id)?.name).toBe( "Renamed", ); @@ -300,20 +330,40 @@ describe("concurrent account-mutating messages", () => { it("stays locked when a sign-out lands during an import", async () => { const key = Keypair.random(); + const active = activeKey(); - const importing = importKey(key.secret()); - const signingOut = send({ type: SERVICE_TYPES.SIGN_OUT }); - await Promise.all([importing, signingOut]); + const importing = importKey(key.secret(), active); + const signingOut = send({ + type: SERVICE_TYPES.SIGN_OUT, + activePublicKey: active, + }); + const [, signOutResult] = await Promise.all([importing, signingOut]); + expect(signOutResult.error).toBeUndefined(); expect(session().publicKey).toBeFalsy(); }); + it("still refuses a signing request carrying the key an import replaced", async () => { + const stale = activeKey(); + const key = Keypair.random(); + + await importKey(key.secret(), stale); + expect(activeKey()).toBe(key.publicKey()); + + const result = await send({ + type: SERVICE_TYPES.SIGN_TRANSACTION, + activePublicKey: stale, + }); + + expect(result.error).toBe("Public key does not match active public key"); + }); + it("answers a read-only message without waiting for an in-flight import", async () => { const order: string[] = []; const key = Keypair.random(); await Promise.all([ - importKey(key.secret()).then(() => order.push("import")), + importKey(key.secret(), activeKey()).then(() => order.push("import")), send({ type: SERVICE_TYPES.LOAD_ACCOUNT }).then(() => order.push("load")), ]); diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index fa7539cebb..463a360421 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -150,6 +150,26 @@ startQueueCleanup({ authEntryQueue, }); +/* + Messages that skip the active-key mismatch check. None of them act on the + active key: signing out is safe whichever account is active, and the rest + target an explicit account id, source or new key. Account-changing messages + run one at a time, so one queued behind an import or switch from another + surface sees the new active key, and refusing it would only drop the + request (a refused sign-out leaves the wallet unlocked). +*/ +const ACTIVE_KEY_INDEPENDENT_SERVICE_TYPES: ReadonlySet = + new Set([ + SERVICE_TYPES.GET_IS_ACCOUNT_MISMATCH, + SERVICE_TYPES.SIGN_OUT, + SERVICE_TYPES.UPDATE_ACCOUNT_NAME, + SERVICE_TYPES.MAKE_ACCOUNT_ACTIVE, + SERVICE_TYPES.ADD_ACCOUNT, + SERVICE_TYPES.IMPORT_ACCOUNT, + SERVICE_TYPES.IMPORT_HARDWARE_WALLET, + SERVICE_TYPES.ADD_RECOVERY_PHRASE_SOURCE, + ]); + export const popupMessageListener = ( request: ServiceMessageRequest, sessionStore: Store, @@ -195,7 +215,7 @@ export const popupMessageListener = ( if ( request.activePublicKey && request.activePublicKey !== publicKey && - request.type !== SERVICE_TYPES.GET_IS_ACCOUNT_MISMATCH + !ACTIVE_KEY_INDEPENDENT_SERVICE_TYPES.has(request.type) ) { return { error: "Public key does not match active public key" }; } diff --git a/extension/src/popup/ducks/__tests__/accountServices.test.ts b/extension/src/popup/ducks/__tests__/accountServices.test.ts index f98bd4b1ea..b8d3dc878c 100644 --- a/extension/src/popup/ducks/__tests__/accountServices.test.ts +++ b/extension/src/popup/ducks/__tests__/accountServices.test.ts @@ -9,6 +9,7 @@ import { hardwareWalletTypeSelector, lockAccount, makeAccountActive, + signOut, } from "../accountServices"; const makeStore = () => @@ -135,3 +136,32 @@ describe("active account with a shared address", () => { ); }); }); + +describe("signOut", () => { + afterEach(() => jest.restoreAllMocks()); + + it("rejects when the background refuses the sign-out", async () => { + const store = makeStore(); + jest.spyOn(ApiInternal, "signOut").mockResolvedValue({ + error: "Public key does not match active public key", + } as any); + + const action = await (store.dispatch as any)(signOut()); + + expect(action.type).toBe(signOut.rejected.type); + expect(store.getState().auth.publicKey).toBe("GBTEST"); + }); + + it("clears the session when the background signs out", async () => { + const store = makeStore(); + jest.spyOn(ApiInternal, "signOut").mockResolvedValue({ + publicKey: "", + applicationState: "MNEMONIC_PHRASE_CONFIRMED", + } as any); + + const action = await (store.dispatch as any)(signOut()); + + expect(action.type).toBe(signOut.fulfilled.type); + expect(store.getState().auth.publicKey).toBe(""); + }); +}); diff --git a/extension/src/popup/ducks/accountServices.ts b/extension/src/popup/ducks/accountServices.ts index 2862a31f58..e655c74632 100644 --- a/extension/src/popup/ducks/accountServices.ts +++ b/extension/src/popup/ducks/accountServices.ts @@ -507,7 +507,9 @@ export const signOut = createAsyncThunk< console.error(e); } - if (res?.publicKey) { + // A refusal carries `error` and no `publicKey`; treating it as success + // would show the lock screen while the background stays unlocked. + if (res?.publicKey || (res && "error" in res)) { return rejectWithValue({ errorMessage: i18n.t("Unable to sign out"), }); From 9e55ddc8a0e39529b47a957a7f93f149782d9e8d Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 11 Nov 2026 13:55:50 -0700 Subject: [PATCH 3/6] fix(accounts): drop deprecated account subscription, bound recovery lookups --- extension/e2e-tests/helpers/stubs.ts | 8 ---- extension/src/background/ducks/session.ts | 6 +-- .../helpers/account-mutation-lock.ts | 4 ++ extension/src/background/helpers/account.ts | 46 +------------------ .../__tests__/recoverAccount.test.ts | 29 ++++++++++++ .../messageListener/handlers/changeNetwork.ts | 10 +--- .../handlers/recoverAccount.ts | 6 +++ .../messageListener/popupMessageListener.ts | 1 - 8 files changed, 45 insertions(+), 65 deletions(-) diff --git a/extension/e2e-tests/helpers/stubs.ts b/extension/e2e-tests/helpers/stubs.ts index 57e6aed30f..fbf218ebdf 100644 --- a/extension/e2e-tests/helpers/stubs.ts +++ b/extension/e2e-tests/helpers/stubs.ts @@ -66,14 +66,6 @@ export const stubRpcHealth = async (context: BrowserContext) => { }); }; -export const stubSubscriptionAccount = async (context: BrowserContext) => { - await context.route("*/**/subscription/account", async (route) => { - await route.fulfill({ - json: { data: {}, error: null }, - }); - }); -}; - export const stubStellarAssetList = async (page: Page) => { await page.route("*/**/testnet/asset-list/**", async (route) => { await route.fulfill({ json: STELLAR_EXPERT_ASSET_LIST_JSON }); diff --git a/extension/src/background/ducks/session.ts b/extension/src/background/ducks/session.ts index b243f5222a..d49f788697 100644 --- a/extension/src/background/ducks/session.ts +++ b/extension/src/background/ducks/session.ts @@ -7,7 +7,7 @@ import { import { Account, ErrorMessage } from "@shared/api/types"; import { getIsHardwareWalletActive, - subscribeAccount as internalSubscribeAccount, + setLastUsedAccount, } from "background/helpers/account"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { TEMPORARY_STORE_ID } from "constants/localStorageTypes"; @@ -18,7 +18,7 @@ export const logIn = createAsyncThunk< { rejectValue: ErrorMessage } >("logIn", async ({ publicKey, allAccounts, localStore }, thunkApi) => { try { - await internalSubscribeAccount({ publicKey, localStore }); + await setLastUsedAccount({ publicKey, localStore }); return { publicKey, allAccounts, @@ -35,7 +35,7 @@ export const setActivePublicKey = createAsyncThunk< { rejectValue: ErrorMessage } >("setActivePublicKey", async ({ publicKey, localStore }, thunkApi) => { try { - await internalSubscribeAccount({ publicKey, localStore }); + await setLastUsedAccount({ publicKey, localStore }); return { publicKey, privateKey: "", diff --git a/extension/src/background/helpers/account-mutation-lock.ts b/extension/src/background/helpers/account-mutation-lock.ts index 49aa8f134b..97e2ed7569 100644 --- a/extension/src/background/helpers/account-mutation-lock.ts +++ b/extension/src/background/helpers/account-mutation-lock.ts @@ -10,6 +10,10 @@ import { SERVICE_TYPES } from "@shared/constants/services"; Read-only messages stay off the lock so an import's Scrypt doesn't stall them. + + Sign-out and the auto-lock alarm wait on this lock too, so a holder must + not await a network call without a timeout. Best-effort requests should + run detached and write only to storage.local. */ export const ACCOUNT_MUTATING_SERVICE_TYPES: ReadonlySet = new Set([ SERVICE_TYPES.CREATE_ACCOUNT, diff --git a/extension/src/background/helpers/account.ts b/extension/src/background/helpers/account.ts index 1cd8efe96b..6e4aa7af45 100644 --- a/extension/src/background/helpers/account.ts +++ b/extension/src/background/helpers/account.ts @@ -7,7 +7,6 @@ import { NETWORK_ID, NETWORKS_LIST_ID, IS_EXPERIMENTAL_MODE_ID, - HAS_ACCOUNT_SUBSCRIPTION, ASSETS_LISTS_ID, IS_HASH_SIGNING_ENABLED_ID, IS_NON_SSL_ENABLED_ID, @@ -465,56 +464,15 @@ export const getFeatureFlags = async () => { return featureFlags; }; -export const subscribeAccount = async ({ +// Shown as the identicon on the login screen. +export const setLastUsedAccount = async ({ publicKey, localStore, }: { publicKey: string; localStore: DataStorageAccess; }) => { - // update last used account so we can use it to properly - // display the identicon component on login screen await localStore.setItem(LAST_USED_ACCOUNT, publicKey); - - // if pub key already has a subscription setup, skip this - const keyId = await localStore.getItem(KEY_ID); - const hasAccountSubByKeyId = - (await localStore.getItem(HAS_ACCOUNT_SUBSCRIPTION)) || {}; - if (!keyId || hasAccountSubByKeyId[keyId]) { - return { publicKey }; - } - - try { - const networkDetails = await getNetworkDetails({ localStore }); - - const options = { - method: "POST", - headers: { - "Content-Type": "application/json", - }, - body: JSON.stringify({ - pub_key: publicKey, - network: networkDetails.network, - }), - }; - - const res = await fetch(`${INDEXER_URL}/subscription/account`, options); - const subsByKeyId = { - ...hasAccountSubByKeyId, - [keyId]: true, - }; - - if (res.ok) { - await localStore.setItem(HAS_ACCOUNT_SUBSCRIPTION, subsByKeyId); - } else { - const resJson = (await res.json()) as string; - throw new Error(resJson); - } - } catch (e) { - console.error(e); - } - - return { publicKey }; }; export const subscribeTokenBalance = async ({ diff --git a/extension/src/background/messageListener/__tests__/recoverAccount.test.ts b/extension/src/background/messageListener/__tests__/recoverAccount.test.ts index 58cdd43d75..eebc3097e2 100644 --- a/extension/src/background/messageListener/__tests__/recoverAccount.test.ts +++ b/extension/src/background/messageListener/__tests__/recoverAccount.test.ts @@ -134,4 +134,33 @@ describe("Recover account message listener", () => { VECTOR.userId, ); }); + it("gives up on a stalled funded lookup and still recovers index 0", async () => { + // Every lookup stalls until its signal aborts; abort them at once instead + // of waiting out the real timeout. + jest + .spyOn(AbortSignal, "timeout") + .mockImplementation(() => AbortSignal.abort()); + global.fetch = jest.fn( + (_url: string, init?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + if (init?.signal?.aborted) reject(init.signal.reason); + init?.signal?.addEventListener("abort", () => + reject(init.signal?.reason), + ); + }), + ) as unknown as typeof fetch; + + const res = await recover(); + + expect((res as { error?: string }).error).toBeFalsy(); + const { session } = mockSessionStore.getState(); + expect(session.allAccounts?.map(({ publicKey }) => publicKey)).toEqual([ + "pubKey0", + ]); + expect( + (global.fetch as jest.Mock).mock.calls.every( + ([, init]) => init?.signal instanceof AbortSignal, + ), + ).toBe(true); + }); }); diff --git a/extension/src/background/messageListener/handlers/changeNetwork.ts b/extension/src/background/messageListener/handlers/changeNetwork.ts index 441ab3bf8e..7abad9987d 100644 --- a/extension/src/background/messageListener/handlers/changeNetwork.ts +++ b/extension/src/background/messageListener/handlers/changeNetwork.ts @@ -1,33 +1,25 @@ -import { Store } from "redux"; - import { ChangeNetworkMessage } from "@shared/api/types/message-request"; -import { getSavedNetworks, subscribeAccount } from "background/helpers/account"; -import { publicKeySelector } from "background/ducks/session"; +import { getSavedNetworks } from "background/helpers/account"; import { MAINNET_NETWORK_DETAILS } from "@shared/constants/stellar"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { NETWORK_ID } from "constants/localStorageTypes"; export const changeNetwork = async ({ request, - sessionStore, localStore, }: { request: ChangeNetworkMessage; - sessionStore: Store; localStore: DataStorageAccess; }) => { const { networkName } = request; - const currentState = sessionStore.getState(); const savedNetworks = await getSavedNetworks({ localStore }); - const publicKey = publicKeySelector(currentState); const networkDetails = savedNetworks.find( ({ networkName: savedNetworkName }) => savedNetworkName === networkName, ) || MAINNET_NETWORK_DETAILS; await localStore.setItem(NETWORK_ID, networkDetails); - await subscribeAccount({ publicKey, localStore }); const isRpcHealthy = true; diff --git a/extension/src/background/messageListener/handlers/recoverAccount.ts b/extension/src/background/messageListener/handlers/recoverAccount.ts index 44df6472de..4dfafcd5bd 100644 --- a/extension/src/background/messageListener/handlers/recoverAccount.ts +++ b/extension/src/background/messageListener/handlers/recoverAccount.ts @@ -40,6 +40,8 @@ import { linkWalletSources } from "background/helpers/link-sources"; const { fromMnemonic } = StellarHDWallet; +const FUNDED_LOOKUP_TIMEOUT_MS = 5_000; + export const recoverAccount = async ({ request, localStore, @@ -153,8 +155,12 @@ export const recoverAccount = async ({ const publicKey = wallet.getPublicKey(i); const privateKey = wallet.getSecret(i); + // Bounded: this runs under the account-mutation lock, and a stalled + // lookup would hold off sign-out and auto-lock. A timeout lands in + // the catch below and the index is skipped as unfunded. const resp = await fetch( `${MAINNET_NETWORK_DETAILS.networkUrl}/accounts/${publicKey}`, + { signal: AbortSignal.timeout(FUNDED_LOOKUP_TIMEOUT_MS) }, ); const j = await resp.json(); diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 463a360421..82d13a86ab 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -307,7 +307,6 @@ export const popupMessageListener = ( return changeNetwork({ request, localStore, - sessionStore, }); } case SERVICE_TYPES.LOAD_ACCOUNT: { From 60ed02c84848cb0fa6c25e5e3b53f29bea57e42c Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 11 Nov 2026 14:01:25 -0700 Subject: [PATCH 4/6] fix(accounts): flush a locked handler's session even when it throws --- .../concurrentAccountMutations.test.ts | 40 +++++++++++++ extension/src/background/index.ts | 60 ++++++++++--------- 2 files changed, 72 insertions(+), 28 deletions(-) diff --git a/extension/src/background/__tests__/concurrentAccountMutations.test.ts b/extension/src/background/__tests__/concurrentAccountMutations.test.ts index 9f4e776338..4086e01000 100644 --- a/extension/src/background/__tests__/concurrentAccountMutations.test.ts +++ b/extension/src/background/__tests__/concurrentAccountMutations.test.ts @@ -124,6 +124,10 @@ const { initExtensionMessageListener } = require("background/index"); const { getAccountNameList } = require("background/helpers/account"); // eslint-disable-next-line @typescript-eslint/no-var-requires const { dataStorageAccess } = require("background/helpers/dataStorageAccess"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const popupListenerModule = require("background/messageListener/popupMessageListener"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { updateAccountName } = require("background/ducks/session"); const send = (request: object) => mockMessageListener(request, { id: "fake-extension-id" }); @@ -369,4 +373,40 @@ describe("concurrent account-mutating messages", () => { expect(order).toEqual(["load", "import"]); }); + it("flushes a holder's session before release even when its handler throws", async () => { + mockSession.latency.ms = 50; + const [first] = allAccounts(); + const key = Keypair.random(); + const active = activeKey(); + // A handler that dispatches and then fails, as confirmPassword can when a + // storage read after its unlock dispatch rejects. + jest + .spyOn(popupListenerModule, "popupMessageListener") + .mockImplementationOnce(async (_request: unknown, sessionStore: any) => { + sessionStore.dispatch( + updateAccountName({ id: first.id, updatedAccountName: "Renamed" }), + ); + throw new Error("boom"); + }); + + const [failed, imported] = await Promise.allSettled([ + send({ + type: SERVICE_TYPES.UPDATE_ACCOUNT_NAME, + activePublicKey: active, + id: first.id, + accountName: "Renamed", + }), + importKey(key.secret(), active), + ]); + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(failed.status).toBe("rejected"); + expect(imported.status).toBe("fulfilled"); + expect(allAccounts().find(({ id }) => id === first.id)?.name).toBe( + "Renamed", + ); + expect( + allAccounts().find(({ publicKey }) => publicKey === key.publicKey()), + ).toBeDefined(); + }); }); diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 170a73183c..25b565f587 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -188,34 +188,38 @@ const handleExtensionMessage = async ( const req = request as ExternalRequest | Response; let res; - if (Object.values(SERVICE_TYPES).includes(req.type as SERVICE_TYPES)) { - res = await popupMessageListener( - req as ServiceMessageRequest, - sessionStore, - localStore, - keyManager, - sessionTimer, - sender, - ); - } - if ( - Object.values(EXTERNAL_SERVICE_TYPES).includes( - req.type as EXTERNAL_SERVICE_TYPES, - ) - ) { - res = await freighterApiMessageListener( - req as ExternalRequest, - sender, - sessionStore, - localStore, - ); - } - - if (flushBeforeRelease) { - // The store's subscribe flush is fire-and-forget. Wait for the final - // state to land before the lock passes to the next holder, which - // loads the store from it. - await flushSessionStore(sessionStore); + try { + if (Object.values(SERVICE_TYPES).includes(req.type as SERVICE_TYPES)) { + res = await popupMessageListener( + req as ServiceMessageRequest, + sessionStore, + localStore, + keyManager, + sessionTimer, + sender, + ); + } + if ( + Object.values(EXTERNAL_SERVICE_TYPES).includes( + req.type as EXTERNAL_SERVICE_TYPES, + ) + ) { + res = await freighterApiMessageListener( + req as ExternalRequest, + sender, + sessionStore, + localStore, + ); + } + } finally { + if (flushBeforeRelease) { + // The store's subscribe flush is fire-and-forget. Wait for the final + // state to land before the lock passes to the next holder, which + // loads the store from it. Also on a throw: a handler can dispatch + // before it fails. flushSessionStore swallows its own errors, so the + // handler's error still reaches the caller. + await flushSessionStore(sessionStore); + } } return res; From ee17cfc6e93ac0cd66b6e6a0fa1d3241cd05c832 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 11 Nov 2026 14:09:40 -0700 Subject: [PATCH 5/6] fix(accounts): skip a queued auto-lock after the session is renewed --- .../concurrentAccountMutations.test.ts | 44 +++++++++++++++++-- .../__tests__/initAlarmListener.test.ts | 15 +++++++ extension/src/background/index.ts | 13 +++++- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/extension/src/background/__tests__/concurrentAccountMutations.test.ts b/extension/src/background/__tests__/concurrentAccountMutations.test.ts index 4086e01000..9f36982bc6 100644 --- a/extension/src/background/__tests__/concurrentAccountMutations.test.ts +++ b/extension/src/background/__tests__/concurrentAccountMutations.test.ts @@ -61,6 +61,9 @@ const makeArea = () => { const mockLocal = makeArea(); const mockSession = makeArea(); let mockMessageListener: (request: unknown, sender: unknown) => Promise; +// Pending alarms by name. A one-shot alarm is removed when it fires. +const mockAlarms = new Set(); +let mockAlarmListener: (alarm: { name: string }) => Promise; jest.mock("webextension-polyfill", () => ({ __esModule: true, @@ -84,9 +87,18 @@ jest.mock("webextension-polyfill", () => ({ }, }, alarms: { - create: jest.fn().mockResolvedValue(undefined), - clear: jest.fn().mockResolvedValue(true), - onAlarm: { addListener: jest.fn() }, + create: jest.fn(async (name: string) => { + mockAlarms.add(name); + }), + get: jest.fn(async (name: string) => + mockAlarms.has(name) ? { name, scheduledTime: 0 } : undefined, + ), + clear: jest.fn(async (name: string) => mockAlarms.delete(name)), + onAlarm: { + addListener: (fn: typeof mockAlarmListener) => { + mockAlarmListener = fn; + }, + }, }, }, })); @@ -119,7 +131,12 @@ if (!global.crypto.randomUUID) { } // eslint-disable-next-line @typescript-eslint/no-var-requires -const { initExtensionMessageListener } = require("background/index"); +const { + initExtensionMessageListener, + initAlarmListener, +} = require("background/index"); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { SESSION_ALARM_NAME } = require("background/helpers/session"); // eslint-disable-next-line @typescript-eslint/no-var-requires const { getAccountNameList } = require("background/helpers/account"); // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -187,6 +204,7 @@ describe("concurrent account-mutating messages", () => { beforeAll(() => { initExtensionMessageListener(); + initAlarmListener(); }); beforeEach(async () => { @@ -194,6 +212,7 @@ describe("concurrent account-mutating messages", () => { await mockLocal.clear(); await mockSession.clear(); mockGenerated.length = 0; + mockAlarms.clear(); global.fetch = jest.fn(() => Promise.resolve({ json: () => Promise.resolve({ account_id: "funded" }), @@ -409,4 +428,21 @@ describe("concurrent account-mutating messages", () => { allAccounts().find(({ publicKey }) => publicKey === key.publicKey()), ).toBeDefined(); }); + it("skips a queued auto-lock when activity rearms the timer", async () => { + const key = Keypair.random(); + const active = activeKey(); + + const importing = importKey(key.secret(), active); + // The idle alarm fires while the import holds the lock: it's removed, + // and its callback queues behind the import. + mockAlarms.delete(SESSION_ALARM_NAME); + const autoLock = mockAlarmListener({ name: SESSION_ALARM_NAME }); + // Activity rearms the timer before the callback gets the lock. + const activity = send({ type: SERVICE_TYPES.USER_ACTIVITY }); + await Promise.all([importing, autoLock, activity]); + + // The idle lock clears the hash key; publicKey survives it. + expect(session().hashKey?.key).toBeTruthy(); + expect(local(TEMPORARY_STORE_ID)).toBeDefined(); + }); }); diff --git a/extension/src/background/__tests__/initAlarmListener.test.ts b/extension/src/background/__tests__/initAlarmListener.test.ts index 7fd17c8991..84509e7d1a 100644 --- a/extension/src/background/__tests__/initAlarmListener.test.ts +++ b/extension/src/background/__tests__/initAlarmListener.test.ts @@ -7,9 +7,11 @@ let alarmHandler: | undefined; const mockSendMessage = jest.fn().mockResolvedValue(undefined); +const mockGetAlarm = jest.fn().mockResolvedValue(undefined); jest.mock("webextension-polyfill", () => ({ alarms: { + get: (...args: any[]) => mockGetAlarm(...args), onAlarm: { addListener: jest.fn((handler) => { alarmHandler = handler; @@ -49,6 +51,7 @@ describe("initAlarmListener", () => { beforeEach(() => { mockSendMessage.mockClear(); mockClearSession.mockClear(); + mockGetAlarm.mockReset().mockResolvedValue(undefined); alarmHandler = undefined; initAlarmListener(); }); @@ -86,4 +89,16 @@ describe("initAlarmListener", () => { expect(mockClearSession).not.toHaveBeenCalled(); expect(mockSendMessage).not.toHaveBeenCalled(); }); + it("skips a firing that a rearm superseded while it waited", async () => { + mockGetAlarm.mockResolvedValueOnce({ + name: SESSION_ALARM_NAME, + scheduledTime: Date.now() + 60_000, + }); + + await alarmHandler!({ name: SESSION_ALARM_NAME }); + + expect(mockGetAlarm).toHaveBeenCalledWith(SESSION_ALARM_NAME); + expect(mockClearSession).not.toHaveBeenCalled(); + expect(mockSendMessage).not.toHaveBeenCalled(); + }); }); diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 25b565f587..0f22c476be 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -271,12 +271,21 @@ export const initAlarmListener = () => { } // Locking mid-import must not be undone by the import's later flush. - await withAccountMutationLock(async () => { + const cleared = await withAccountMutationLock(async () => { + // A fired alarm is gone, so a pending one means resetSession or + // startSession rearmed the timer while this one waited for the lock. + // The session it would clear has a new deadline. + if (await browser.alarms.get(SESSION_ALARM_NAME)) { + return false; + } const sessionStore = await buildStore(); const localStore = dataStorageAccess(browserLocalStorage); await clearSession({ sessionStore, localStore }); await flushSessionStore(sessionStore); + return true; }); - await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); + if (cleared) { + await broadcastSessionState(SERVICE_TYPES.SESSION_LOCKED); + } }); }; From 08ee53232dcb7bcc45e14ad144361204f2f430ad Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 11 Nov 2026 14:19:19 -0700 Subject: [PATCH 6/6] test(e2e): remove call to deleted subscription account stub --- extension/e2e-tests/helpers/stubs.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/extension/e2e-tests/helpers/stubs.ts b/extension/e2e-tests/helpers/stubs.ts index fbf218ebdf..77aa7007bb 100644 --- a/extension/e2e-tests/helpers/stubs.ts +++ b/extension/e2e-tests/helpers/stubs.ts @@ -3384,9 +3384,6 @@ export const stubAllExternalApis = async ( // RPC health is fetched by the background service worker await stubRpcHealth(context); - // Subscription account - await stubSubscriptionAccount(context); - // Stellar Asset List await stubStellarAssetList(page); // Asset search is fetched by the background service worker