From 1d2c11702ea7e05127c6f06b436e94710e429f47 Mon Sep 17 00:00:00 2001 From: ost-ptk Date: Tue, 25 Aug 2026 13:08:35 +0300 Subject: [PATCH 1/3] fix(background): cancel open requests and clear the mirror on wallet reset --- src/background/handlers/cancel-requests.ts | 95 +++--- .../handlers/redux-actions.parity.test.ts | 6 +- .../redux/sagas/onboarding-sagas.test.ts | 274 +++++++++++++++++- .../redux/sagas/onboarding-sagas.ts | 72 ++++- .../redux/windowManagement/actions.ts | 1 + .../redux/windowManagement/reducer.test.ts | 25 ++ .../redux/windowManagement/reducer.ts | 10 +- .../windowManagement/session-store.test.ts | 30 ++ .../redux/windowManagement/session-store.ts | 10 + 9 files changed, 481 insertions(+), 42 deletions(-) diff --git a/src/background/handlers/cancel-requests.ts b/src/background/handlers/cancel-requests.ts index 2f60eb710..285bd6e11 100644 --- a/src/background/handlers/cancel-requests.ts +++ b/src/background/handlers/cancel-requests.ts @@ -196,41 +196,21 @@ export async function cancelRequestsDisplacedBy( await cancelRequests(store, candidates, source, windowId, afterMark); } -// The trigger with no window event behind it — either `windows.create` -// rejected (no `windows.onRemoved` will ever fire for a window that never -// existed) or the startup sweep decided a hydrated 'open' row is orphaned -// (spec §8.1). Without this the dapp promise hangs until its own timeout -// (30 min by default). -// -// `source` defaults to the original trigger so the `open-window-failed` call -// site keeps its BANNER policy unchanged — its delivery does not: #1484's -// stale-tab check below applies to every source alike, so that call site's -// delivery is deliberately narrowed too, same as the sweep's. `source` doubles -// as the banner policy: only -// `'open-window-failed'` dispatches `sagaError` (a `windows.create` failure is -// the wallet's own doing, with nothing else to tell the user). Every other -// source — today just the sweep — is dapp-triggerable and console-only, -// matching the precedent in `sdk-methods.ts`'s `reportCapacityRefusal`: a -// banner mounted route-independently over every approval screen must not -// fire for a request the user cannot act on, and in close-as-wake the sweep's -// enumeration resolves inside another cancel's own grace, so it would often -// paint over an ordinary close or a live signing prompt. -export async function failRequestOnWindowError( - store: MainStore, - requestId: string, - source: SagaErrorSource = 'open-window-failed' -): Promise { - const request = selectOpenRequests(store.getState()).find( - openRequest => openRequest.requestId === requestId - ); - - if (request == null) { - return; - } - - store.dispatch(windowRequestResponded({ requestId })); +// The store-free half of `failRequestOnWindowError`: origin check, direct +// send, `deliverViaOrigin` fallback. Takes a snapshot row rather than reading +// the store, so it works from a saga (no store access there) as much as from +// a handler that has one — `resetVaultSaga`'s cancel-then-clear (spec §8.3) +// shares it with the caller below. +export type CancelDeliveryRow = Pick< + OpenRequest, + 'requestId' | 'tabId' | 'origin' | 'method' | 'frameId' +>; - const { tabId, origin, method, frameId } = request; +export async function deliverCancelResponse( + row: CancelDeliveryRow, + logSource: string +): Promise { + const { requestId, tabId, origin, method, frameId } = row; const action = buildCancelResponse(method, requestId); // #1484: verify the tab still hosts the requesting origin BEFORE sending. @@ -253,7 +233,7 @@ export async function failRequestOnWindowError( // Identifiers and origins only, matching sdk-response-to-tab's withheld- // response log — never a URL. console.error( - `${source}: target tab no longer hosts the requesting origin; response withheld`, + `${logSource}: target tab no longer hosts the requesting origin; response withheld`, { requestId, tabId, expectedOrigin: origin, liveOrigin, delivered } ); } else { @@ -265,7 +245,7 @@ export async function failRequestOnWindowError( // Never the raw error: a `tabs.sendMessage` rejection can echo back a // navigated-away tab's URL, and one of this window's own URLs carries a // signMessage request's plaintext message as a query param. - console.error(`${source}: cancel delivery failed`, { + console.error(`${logSource}: cancel delivery failed`, { requestId, method, tabId, @@ -275,6 +255,49 @@ export async function failRequestOnWindowError( } } + return delivered; +} + +// The trigger with no window event behind it — either `windows.create` +// rejected (no `windows.onRemoved` will ever fire for a window that never +// existed) or the startup sweep decided a hydrated 'open' row is orphaned +// (spec §8.1). Without this the dapp promise hangs until its own timeout +// (30 min by default). +// +// `source` defaults to the original trigger so the `open-window-failed` call +// site keeps its BANNER policy unchanged — its delivery does not: #1484's +// stale-tab check below applies to every source alike, so that call site's +// delivery is deliberately narrowed too, same as the sweep's. `source` doubles +// as the banner policy: only +// `'open-window-failed'` dispatches `sagaError` (a `windows.create` failure is +// the wallet's own doing, with nothing else to tell the user). Every other +// source — today just the sweep — is dapp-triggerable and console-only, +// matching the precedent in `sdk-methods.ts`'s `reportCapacityRefusal`: a +// banner mounted route-independently over every approval screen must not +// fire for a request the user cannot act on, and in close-as-wake the sweep's +// enumeration resolves inside another cancel's own grace, so it would often +// paint over an ordinary close or a live signing prompt. +export async function failRequestOnWindowError( + store: MainStore, + requestId: string, + source: SagaErrorSource = 'open-window-failed' +): Promise { + const request = selectOpenRequests(store.getState()).find( + openRequest => openRequest.requestId === requestId + ); + + if (request == null) { + return; + } + + store.dispatch(windowRequestResponded({ requestId })); + + const { tabId, origin, method, frameId } = request; + const delivered = await deliverCancelResponse( + { requestId, tabId, origin, method, frameId }, + source + ); + // The sweep (source === 'sweep-orphaned-requests') knowingly shares this // same tombstone-before-delivery ordering — an accepted residual, not an // oversight specific to the sweep. diff --git a/src/background/handlers/redux-actions.parity.test.ts b/src/background/handlers/redux-actions.parity.test.ts index 44d869bca..4eece714e 100644 --- a/src/background/handlers/redux-actions.parity.test.ts +++ b/src/background/handlers/redux-actions.parity.test.ts @@ -205,7 +205,11 @@ const EXCLUSIONS: ReadonlySet = new Set( // Background-only since WALLET-1424: the background owns the retry counter, // so a page can no longer forge attempts or clear the count. loginRetryCountActions.loginRetryCountIncremented, - loginRetryCountActions.loginRetryCountReseted + loginRetryCountActions.loginRetryCountReseted, + // Background-only (spec §8.3): `yield put` inside `resetVaultSaga` only, as + // part of the synchronous reset block. Never dispatched from the UI — a + // saga `put` never reaches `handleReduxAction` at all. + windowManagementActions.windowManagementReseted ].map(creator => creator.type) ); diff --git a/src/background/redux/sagas/onboarding-sagas.test.ts b/src/background/redux/sagas/onboarding-sagas.test.ts index f58dc037e..d400f3194 100644 --- a/src/background/redux/sagas/onboarding-sagas.test.ts +++ b/src/background/redux/sagas/onboarding-sagas.test.ts @@ -1,13 +1,23 @@ import { combineReducers } from '@reduxjs/toolkit'; +import { Middleware, UnknownAction, applyMiddleware, createStore } from 'redux'; +import createSagaMiddleware from 'redux-saga'; import { expectSaga } from 'redux-saga-test-plan'; +import { storage, windows } from 'webextension-polyfill'; + +import { deliverCancelResponse } from '@background/handlers/cancel-requests'; +import { vaultReseted } from '@background/redux/vault/actions'; +import { windowManagementReseted } from '@background/redux/windowManagement/actions'; +import { reducer as windowManagementReducer } from '@background/redux/windowManagement/reducer'; +import { clearRequestSession } from '@background/redux/windowManagement/session-store'; import { reducer as keysReducer } from '../keys/reducer'; import { reducer as sessionReducer } from '../session/reducer'; -import { initKeys } from './actions'; +import { initKeys, resetVault } from './actions'; import { onboardingSagas } from './onboarding-sagas'; jest.mock('webextension-polyfill', () => ({ - storage: { local: { clear: jest.fn() } } + storage: { local: { clear: jest.fn() } }, + windows: { remove: jest.fn() } })); jest.mock('@background/open-onboarding-flow', () => ({ disableOnboardingFlow: jest.fn() @@ -19,6 +29,12 @@ jest.mock('@background/workers/scrypt-off-thread', () => ({ encodePasswordOffThread: jest.fn().mockResolvedValue('password-hash'), deriveScryptKey: jest.fn().mockResolvedValue(new Uint8Array([1, 2, 3])) })); +jest.mock('@background/handlers/cancel-requests', () => ({ + deliverCancelResponse: jest.fn() +})); +jest.mock('@background/redux/windowManagement/session-store', () => ({ + clearRequestSession: jest.fn() +})); const rootReducer = combineReducers({ keys: keysReducer, @@ -60,3 +76,257 @@ it('never leaves keys visible without a session while creating them', async () = expect(storeState.keys.keysDoesExist).toBe(true); expect(storeState.session.encryptionKeyDoesExist).toBe(true); }); + +/** + * spec §8.3 — cancel-then-clear on wallet reset. The resets and + * `storage.local.clear()` must complete synchronously inside the saga, before + * anything the saga does not await (delivery, `windows.remove`, the session + * mirror clear) has a chance to run — a slow or rejecting delivery must never + * hold up the reset itself. + */ +describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () => { + const openRequest = { + status: 'open' as const, + tabId: 3, + origin: 'https://dapp', + method: 'sign' as const, + windowIds: [42], + awaitingDeviceConfirmation: false, + seq: 0 + }; + + const stateWithOneOpenRequest = { + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: { r1: openRequest } + } + }; + + const countPutsOfType = (allEffects: unknown, type: string) => + ( + allEffects as Array<{ + type: string; + payload?: { action?: { type?: string } }; + }> + ).filter( + effect => effect.type === 'PUT' && effect.payload?.action?.type === type + ).length; + + beforeEach(() => { + jest.clearAllMocks(); + (clearRequestSession as jest.Mock).mockResolvedValue(undefined); + (windows.remove as jest.Mock).mockResolvedValue(undefined); + }); + + it('completes every reset and storage.local.clear() synchronously, even though delivery never resolves', async () => { + (deliverCancelResponse as jest.Mock).mockReturnValue(new Promise(() => {})); + + const { allEffects } = await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + + expect(countPutsOfType(allEffects, vaultReseted.type)).toBe(1); + expect(countPutsOfType(allEffects, windowManagementReseted.type)).toBe(1); + expect(storage.local.clear).toHaveBeenCalled(); + }); + + it('completes every reset and storage.local.clear() synchronously, even though delivery rejects', async () => { + (deliverCancelResponse as jest.Mock).mockRejectedValue( + new Error('delivery failed') + ); + + const { allEffects } = await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + + expect(countPutsOfType(allEffects, vaultReseted.type)).toBe(1); + expect(countPutsOfType(allEffects, windowManagementReseted.type)).toBe(1); + expect(storage.local.clear).toHaveBeenCalled(); + }); + + // `expectSaga(...).silentRun(50)` above only pins that the resets and + // `storage.local.clear()` complete WITHIN 50ms of the dispatch — it would + // stay green even if a `yield delay(0)` (or any other awaited effect) were + // inserted ABOVE the first reset `put`, because 0ms/near-0ms async work + // still resolves well inside a 50ms window. That is exactly the Firefox/ + // Safari regression spec §8.3 describes: on those targets the UI's + // `.then(() => closeWindowByReloadExtension())` — `runtime.reload()` — races + // the FIRST microtask/macrotask boundary after `store.dispatch(resetVault())` + // returns, not a 50ms deadline. Only a real store + real saga middleware, + // asserted on the very next synchronous line with no `await`, proves the + // resets land inside the same synchronous flush as the dispatch call itself + // (redux-saga drains `put`/`select` effects synchronously at semaphore 0, + // before yielding back to the caller of `dispatch`). + it('lands every reset and storage.local.clear() SYNCHRONOUSLY inside store.dispatch(resetVault()) — before delivery, which never resolves, could ever run', () => { + (deliverCancelResponse as jest.Mock).mockReturnValue(new Promise(() => {})); + (windows.remove as jest.Mock).mockReturnValue(new Promise(() => {})); + (clearRequestSession as jest.Mock).mockReturnValue(new Promise(() => {})); + + const dispatchedTypes: string[] = []; + const loggerMiddleware: Middleware = () => next => (action: unknown) => { + dispatchedTypes.push((action as UnknownAction).type); + return next(action); + }; + + const sagaMiddleware = createSagaMiddleware(); + const store = createStore( + combineReducers({ windowManagement: windowManagementReducer }), + { windowManagement: stateWithOneOpenRequest.windowManagement }, + applyMiddleware(loggerMiddleware, sagaMiddleware) + ); + sagaMiddleware.run(onboardingSagas); + + store.dispatch(resetVault()); + + // No `await`, no fake/real timers — this is the very next synchronous + // statement after `dispatch` returned. + expect(dispatchedTypes).toContain(vaultReseted.type); + expect(dispatchedTypes).toContain(windowManagementReseted.type); + expect(storage.local.clear).toHaveBeenCalled(); + expect(store.getState().windowManagement).toEqual({ + windowId: null, + exportKeysWindowId: null, + requests: {} + }); + }); + + it('delivers the cancel for an open request at reset time, from the pre-reset snapshot', async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + + await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + + expect(deliverCancelResponse).toHaveBeenCalledWith( + expect.objectContaining({ + requestId: 'r1', + tabId: 3, + origin: 'https://dapp', + method: 'sign' + }), + 'resetVaultSaga' + ); + }); + + it("removes the open request's window", async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + + await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + + expect(windows.remove).toHaveBeenCalledWith(42); + }); + + it('clears the session mirror directly, joining the session-store write chain rather than relying on the subscriber guard', async () => { + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: {} + } + }) + .dispatch(resetVault()) + .silentRun(50); + + expect(clearRequestSession).toHaveBeenCalled(); + }); + + it('no open requests → no delivery, no window removal, mirror still cleared', async () => { + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: {} + } + }) + .dispatch(resetVault()) + .silentRun(50); + + expect(deliverCancelResponse).not.toHaveBeenCalled(); + expect(windows.remove).not.toHaveBeenCalled(); + expect(clearRequestSession).toHaveBeenCalled(); + }); + + it('logs, and does not throw, when a delivery rejects', async () => { + (deliverCancelResponse as jest.Mock).mockRejectedValue( + new Error('deliver failed') + ); + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + await Promise.resolve(); + await Promise.resolve(); + + expect(consoleError).toHaveBeenCalledWith( + 'resetVaultSaga: cancel delivery failed', + { requestId: 'r1' }, + expect.anything() + ); + consoleError.mockRestore(); + }); + + it('logs, and does not throw, when windows.remove rejects', async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + (windows.remove as jest.Mock).mockRejectedValue( + new Error('no such window') + ); + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + await expectSaga(onboardingSagas) + .withState(stateWithOneOpenRequest) + .dispatch(resetVault()) + .silentRun(50); + await Promise.resolve(); + await Promise.resolve(); + + expect(consoleError).toHaveBeenCalledWith( + 'resetVaultSaga: window removal failed', + { windowId: 42 }, + expect.anything() + ); + consoleError.mockRestore(); + }); + + it('logs, and does not throw, when clearing the session mirror rejects', async () => { + (clearRequestSession as jest.Mock).mockRejectedValue( + new Error('write failed') + ); + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: {} + } + }) + .dispatch(resetVault()) + .silentRun(50); + await Promise.resolve(); + await Promise.resolve(); + + expect(consoleError).toHaveBeenCalledWith( + 'resetVaultSaga: clear request mirror failed', + expect.anything() + ); + consoleError.mockRestore(); + }); +}); diff --git a/src/background/redux/sagas/onboarding-sagas.ts b/src/background/redux/sagas/onboarding-sagas.ts index 50f7d0bc3..64a24d2eb 100644 --- a/src/background/redux/sagas/onboarding-sagas.ts +++ b/src/background/redux/sagas/onboarding-sagas.ts @@ -1,9 +1,11 @@ -import { put, takeLatest } from 'redux-saga/effects'; -import { storage } from 'webextension-polyfill'; +import { put, select, takeLatest } from 'redux-saga/effects'; +import { storage, windows } from 'webextension-polyfill'; import { ErrorMessages } from '@src/constants'; +import { deliverCancelResponse } from '@background/handlers/cancel-requests'; import { disableOnboardingFlow } from '@background/open-onboarding-flow'; +import { redactUrlQuery } from '@background/redact-url-query'; import { resetAppEventsDismission, sagaError @@ -13,6 +15,10 @@ import { resetRateApp } from '@background/redux/rate-app/actions'; import { recipientPublicKeyReseted } from '@background/redux/recent-recipient-public-keys/actions'; import { vaultSettingsReseted } from '@background/redux/settings/actions'; import { resetTrustedWasmState } from '@background/redux/trusted-wasm/actions'; +import { windowManagementReseted } from '@background/redux/windowManagement/actions'; +import { selectOpenRequests } from '@background/redux/windowManagement/selectors'; +import { clearRequestSession } from '@background/redux/windowManagement/session-store'; +import { OpenRequest } from '@background/redux/windowManagement/types'; import { deriveScryptKey, encodePasswordOffThread @@ -48,11 +54,55 @@ export function* onboardingSagas() { yield takeLatest(recoverVault.type, recoverVaultSaga); } +// Fire-and-forget: delivery and window cleanup run AFTER the resets below +// have already completed, from a snapshot taken before any of them. Never +// awaited by the saga — see the ordering note there. `failRequestOnWindowError` +// cannot be reused directly here: it needs the store (to dispatch the +// tombstone), which the wallet no longer has any use for once every slice is +// already wiped, so this only needs `deliverCancelResponse`, the store-free +// half it shares. +function deliverResetCancels(openRequests: readonly OpenRequest[]): void { + for (const request of openRequests) { + deliverCancelResponse(request, 'resetVaultSaga').catch(error => { + console.error( + 'resetVaultSaga: cancel delivery failed', + { requestId: request.requestId }, + redactUrlQuery(error) + ); + }); + } + + const windowIds = [...new Set(openRequests.flatMap(r => r.windowIds))]; + + for (const windowId of windowIds) { + windows.remove(windowId).catch(error => { + console.error( + 'resetVaultSaga: window removal failed', + { windowId }, + redactUrlQuery(error) + ); + }); + } +} + /** * */ function* resetVaultSaga() { try { + // Snapshotted BEFORE any reset: the reducer that clears `windowManagement` + // below throws away the descriptors this needs to cancel and to find the + // approval windows to close. + const openRequests: OpenRequest[] = yield select(selectOpenRequests); + + // Order matters and is the whole point (spec §8.3). Today the twelve + // `put`s below complete synchronously inside `store.dispatch(resetVault())` + // — before `handleReduxAction` responds and before the UI's + // `.then(() => closeWindowByReloadExtension())` runs, which on Firefox and + // Safari is `runtime.reload()`. Any awaited I/O ahead of them would let + // that reload kill the saga first, so the resets and `storage.local.clear()` + // would never run. Everything below this comment through `storage.local + // .clear()` MUST stay synchronous — no `yield call`/`yield` on a Promise. yield put(vaultReseted()); yield put(vaultCipherReseted()); yield put(keysReseted()); @@ -65,8 +115,26 @@ function* resetVaultSaga() { yield put(vaultSettingsReseted()); yield put(resetRateApp()); yield put(resetAppEventsDismission()); + yield put(windowManagementReseted()); storage.local.clear(); + + // The reducer above returns the shared `initialState` reference, so when + // `windowManagement` was already at rest the subscriber's identity guard + // (get-main-store.ts) sees no change and never persists the clear. Join + // the write chain directly instead of relying on it. + clearRequestSession().catch(error => { + console.error( + 'resetVaultSaga: clear request mirror failed', + redactUrlQuery(error) + ); + }); + + // Deliveries and window removal happen strictly AFTER the synchronous + // block above, from the snapshot. A slow or rejecting delivery must not + // delay or break the resets or `storage.local.clear()` — it can't, since + // this call is not awaited. + deliverResetCancels(openRequests); } catch (err) { console.error(err); yield put( diff --git a/src/background/redux/windowManagement/actions.ts b/src/background/redux/windowManagement/actions.ts index 40869202f..509f30c25 100644 --- a/src/background/redux/windowManagement/actions.ts +++ b/src/background/redux/windowManagement/actions.ts @@ -9,6 +9,7 @@ export { windowDetachedFromRequests, windowIdChanged, windowIdCleared, + windowManagementReseted, windowRequestDeviceConfirmationChanged, windowRequestOpened, windowRequestResponded, diff --git a/src/background/redux/windowManagement/reducer.test.ts b/src/background/redux/windowManagement/reducer.test.ts index b89be9947..b59d77cc2 100644 --- a/src/background/redux/windowManagement/reducer.test.ts +++ b/src/background/redux/windowManagement/reducer.test.ts @@ -9,6 +9,7 @@ import { windowDetachedFromRequests, windowIdChanged, windowIdCleared, + windowManagementReseted, windowRequestDeviceConfirmationChanged, windowRequestOpened, windowRequestResponded, @@ -116,6 +117,30 @@ describe('windowManagement reducer', () => { state = reducer(state, windowRequestResponded({ requestId: 'r1' })); expect(state.requests.r1).toEqual({ status: 'responded', seq: 2 }); }); + + it('windowManagementReseted wipes windowId, exportKeysWindowId and every request', () => { + let state = reducer(empty, opened('r1')); + state = reducer(state, windowIdChanged(7)); + state = reducer(state, exportKeysWindowIdChanged(12)); + + expect(reducer(state, windowManagementReseted())).toEqual({ + windowId: null, + exportKeysWindowId: null, + requests: {} + }); + }); + + it('windowManagementReseted returns the shared initialState reference', () => { + // Load-bearing for spec §8.3: the get-main-store.ts subscriber guard + // compares `requests`/`windowId` by reference, so a state that was already + // at rest must reset to the SAME object — the reset flow does not rely on + // that guard to persist the clear, but the identity is still the contract + // this reducer promises every other reset case. + const first = reducer(empty, windowManagementReseted()); + const second = reducer(first, windowManagementReseted()); + + expect(first).toBe(second); + }); }); describe('windowManagement requests', () => { diff --git a/src/background/redux/windowManagement/reducer.ts b/src/background/redux/windowManagement/reducer.ts index 729d41eab..020c52f98 100644 --- a/src/background/redux/windowManagement/reducer.ts +++ b/src/background/redux/windowManagement/reducer.ts @@ -226,7 +226,14 @@ const slice = createSlice({ } return { ...state, requests }; - } + }, + // Dispatched only from `resetVaultSaga` (spec §8.3) — never forwarded from + // the UI, see the EXCLUSIONS entry in redux-actions.parity.test.ts. Returns + // the shared `initialState` reference like every other slice's reset case; + // the reset flow does not rely on the subscriber's write guard to clear + // the session mirror for this slice — it clears it directly instead (see + // session-store.ts). + windowManagementReseted: () => initialState } }); @@ -241,6 +248,7 @@ export const { windowDetachedFromRequests, windowIdChanged, windowIdCleared, + windowManagementReseted, windowRequestDeviceConfirmationChanged, windowRequestOpened, windowRequestResponded, diff --git a/src/background/redux/windowManagement/session-store.test.ts b/src/background/redux/windowManagement/session-store.test.ts index 4048df261..cfeda27fc 100644 --- a/src/background/redux/windowManagement/session-store.test.ts +++ b/src/background/redux/windowManagement/session-store.test.ts @@ -2,6 +2,7 @@ import { MAX_RESPONDED_TOMBSTONES } from './reducer'; import { REQUEST_SESSION_KEY, SessionRecord, + clearRequestSession, readRequestSession, writeRequestSession } from './session-store'; @@ -536,6 +537,35 @@ describe('session-store — write path', () => { }); }); +describe('session-store — clearRequestSession (spec §8.3)', () => { + it('writes the empty record under the session key, joining the write chain', async () => { + await clearRequestSession(); + + expect(sessionSet).toHaveBeenCalledWith({ + [REQUEST_SESSION_KEY]: { requests: {}, windowId: null } + }); + }); + + it('coalesces with a same-tick writeRequestSession call onto the newest snapshot', async () => { + void writeRequestSession({ + requests: { 'req-1': openRow() as Request }, + windowId: 3 + }); + await clearRequestSession(); + + expect(sessionSet).toHaveBeenCalledTimes(1); + expect(lastWrittenRecord()).toEqual({ requests: {}, windowId: null }); + }); + + it('is a no-op on a persistent-background build', async () => { + mockIsEphemeralBackgroundBuild = false; + + await clearRequestSession(); + + expect(sessionSet).not.toHaveBeenCalled(); + }); +}); + describe('session-store — logging discipline', () => { it('never logs a raw URL query or the sanitizer input', async () => { sessionGet.mockRejectedValue( diff --git a/src/background/redux/windowManagement/session-store.ts b/src/background/redux/windowManagement/session-store.ts index ff66484db..c47977dd5 100644 --- a/src/background/redux/windowManagement/session-store.ts +++ b/src/background/redux/windowManagement/session-store.ts @@ -307,3 +307,13 @@ export function writeRequestSession(record: SessionRecord): Promise { return writeChain; } + +// A direct clear for callers that cannot rely on the subscriber's +// identity-based write guard (get-main-store.ts) — e.g. a reducer's reset +// case that returns the shared `initialState` reference, which the guard +// sees as no change at all when the slice was already at rest. Goes through +// the same `writeRequestSession`/`writeChain` plumbing, never touching +// `chrome.storage.session` on its own. +export function clearRequestSession(): Promise { + return writeRequestSession(emptyRecord()); +} From 2d0c274f82aaf9e26dae8ff6bc6b2b9a3a40ad20 Mon Sep 17 00:00:00 2001 From: ost-ptk Date: Tue, 25 Aug 2026 14:11:55 +0300 Subject: [PATCH 2/3] test(background): pin reset delivery logging and source type --- src/background/handlers/cancel-requests.ts | 2 +- .../redux/sagas/onboarding-sagas.test.ts | 32 +++++++++++++++---- .../redux/sagas/onboarding-sagas.ts | 2 +- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/background/handlers/cancel-requests.ts b/src/background/handlers/cancel-requests.ts index 285bd6e11..074fee5f4 100644 --- a/src/background/handlers/cancel-requests.ts +++ b/src/background/handlers/cancel-requests.ts @@ -208,7 +208,7 @@ export type CancelDeliveryRow = Pick< export async function deliverCancelResponse( row: CancelDeliveryRow, - logSource: string + logSource: SagaErrorSource ): Promise { const { requestId, tabId, origin, method, frameId } = row; const action = buildCancelResponse(method, requestId); diff --git a/src/background/redux/sagas/onboarding-sagas.test.ts b/src/background/redux/sagas/onboarding-sagas.test.ts index d400f3194..3f2773097 100644 --- a/src/background/redux/sagas/onboarding-sagas.test.ts +++ b/src/background/redux/sagas/onboarding-sagas.test.ts @@ -257,7 +257,7 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () it('logs, and does not throw, when a delivery rejects', async () => { (deliverCancelResponse as jest.Mock).mockRejectedValue( - new Error('deliver failed') + new Error('deliver failed: https://dapp/page?message=super-secret') ); const consoleError = jest .spyOn(console, 'error') @@ -271,17 +271,25 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () await Promise.resolve(); expect(consoleError).toHaveBeenCalledWith( - 'resetVaultSaga: cancel delivery failed', + 'resetVaultSaga: cancel delivery rejected', { requestId: 'r1' }, - expect.anything() + expect.any(String) ); + const [, , loggedError] = consoleError.mock.calls.find( + ([message]) => message === 'resetVaultSaga: cancel delivery rejected' + )!; + // Pins that the argument is `redactUrlQuery`'s output, not the raw + // rejection: a raw `Error` would fail the type check, and an + // un-redacted string would still carry the `?...=` query. + expect(loggedError).not.toBeInstanceOf(Error); + expect(loggedError).not.toMatch(/\?[^"]*=/); consoleError.mockRestore(); }); it('logs, and does not throw, when windows.remove rejects', async () => { (deliverCancelResponse as jest.Mock).mockResolvedValue(1); (windows.remove as jest.Mock).mockRejectedValue( - new Error('no such window') + new Error('no such window: https://dapp/page?message=super-secret') ); const consoleError = jest .spyOn(console, 'error') @@ -297,14 +305,19 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () expect(consoleError).toHaveBeenCalledWith( 'resetVaultSaga: window removal failed', { windowId: 42 }, - expect.anything() + expect.any(String) ); + const [, , loggedError] = consoleError.mock.calls.find( + ([message]) => message === 'resetVaultSaga: window removal failed' + )!; + expect(loggedError).not.toBeInstanceOf(Error); + expect(loggedError).not.toMatch(/\?[^"]*=/); consoleError.mockRestore(); }); it('logs, and does not throw, when clearing the session mirror rejects', async () => { (clearRequestSession as jest.Mock).mockRejectedValue( - new Error('write failed') + new Error('write failed: https://dapp/page?message=super-secret') ); const consoleError = jest .spyOn(console, 'error') @@ -325,8 +338,13 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () expect(consoleError).toHaveBeenCalledWith( 'resetVaultSaga: clear request mirror failed', - expect.anything() + expect.any(String) ); + const [, loggedError] = consoleError.mock.calls.find( + ([message]) => message === 'resetVaultSaga: clear request mirror failed' + )!; + expect(loggedError).not.toBeInstanceOf(Error); + expect(loggedError).not.toMatch(/\?[^"]*=/); consoleError.mockRestore(); }); }); diff --git a/src/background/redux/sagas/onboarding-sagas.ts b/src/background/redux/sagas/onboarding-sagas.ts index 64a24d2eb..3d2782a45 100644 --- a/src/background/redux/sagas/onboarding-sagas.ts +++ b/src/background/redux/sagas/onboarding-sagas.ts @@ -65,7 +65,7 @@ function deliverResetCancels(openRequests: readonly OpenRequest[]): void { for (const request of openRequests) { deliverCancelResponse(request, 'resetVaultSaga').catch(error => { console.error( - 'resetVaultSaga: cancel delivery failed', + 'resetVaultSaga: cancel delivery rejected', { requestId: request.requestId }, redactUrlQuery(error) ); From 1ecadeb8b7b46743f8ca93e05a53574e84ff0078 Mon Sep 17 00:00:00 2001 From: ost-ptk Date: Wed, 26 Aug 2026 09:25:25 +0300 Subject: [PATCH 3/3] fix(background): widen and fence the reset window-removal set --- src/background/handlers/redux-actions.test.ts | 18 ++- src/background/handlers/redux-actions.ts | 9 +- src/background/redux/sagas/actions.ts | 8 +- .../redux/sagas/onboarding-sagas.test.ts | 133 +++++++++++++++++- .../redux/sagas/onboarding-sagas.ts | 101 +++++++++---- 5 files changed, 238 insertions(+), 31 deletions(-) diff --git a/src/background/handlers/redux-actions.test.ts b/src/background/handlers/redux-actions.test.ts index 0196cc898..3a3a9a0df 100644 --- a/src/background/handlers/redux-actions.test.ts +++ b/src/background/handlers/redux-actions.test.ts @@ -74,17 +74,33 @@ beforeEach(() => { describe('handleReduxAction forwarding gate (fail-closed)', () => { it('resetVault → dispatches and re-enables the onboarding flow', async () => { + // A fresh `resetVault(...)` is built here, not the wire action re-cast — + // the sender's own window id (absent for this tab-less sender) is + // attached from `MessageSender`, which the UI has no access to and could + // not be trusted to self-report even if it did. const { store, dispatch } = makeStore(); const action = { type: resetVault.type }; const result = await handleReduxAction(action, trustedSender, store); expect(dispatch).toHaveBeenCalledTimes(1); - expect(dispatch).toHaveBeenCalledWith(action); + expect(dispatch).toHaveBeenCalledWith(resetVault(undefined)); expect(enableOnboardingFlowMock).toHaveBeenCalledTimes(1); expect(result).toEqual({ handled: true, response: undefined }); }); + it('resetVault from a tab sender → attaches the sender window id, excluded later by the saga', async () => { + const { store, dispatch } = makeStore(); + const tabSender = { + ...trustedSender, + tab: { id: 9, windowId: 7 } + } as Runtime.MessageSender; + + await handleReduxAction({ type: resetVault.type }, tabSender, store); + + expect(dispatch).toHaveBeenCalledWith(resetVault(7)); + }); + it('windowRequestWindowAttached → handled by its own branch, which verifies the window', async () => { // It must reach the store (the Ledger hook dispatches it from a UI page), // but through the branch that probes the window rather than through the diff --git a/src/background/handlers/redux-actions.ts b/src/background/handlers/redux-actions.ts index 67fffd5c9..a3e1ac3fb 100644 --- a/src/background/handlers/redux-actions.ts +++ b/src/background/handlers/redux-actions.ts @@ -341,7 +341,14 @@ export async function handleReduxAction( } if (action.type === resetVault.type) { - store.dispatch(action as unknown as ReduxAction); + // The sender's OWN window, from `MessageSender` rather than the wire + // payload — `ResetVaultPage` renders inside the signature-request and + // connect-to-app approval windows (`LockedRouter`), so `resetVaultSaga`'s + // window-removal set must exclude it: closing the window the reset was + // issued FROM would kill the page's own continuation + // (`closeWindowByReloadExtension`), and on Firefox/Safari that also skips + // `runtime.reload()`. Absent for a non-tab sender, hence optional. + store.dispatch(resetVault(sender.tab?.windowId)); await enableOnboardingFlow(); return { handled: true, response: undefined }; } diff --git a/src/background/redux/sagas/actions.ts b/src/background/redux/sagas/actions.ts index fd3b8d479..7ef0dbdaa 100644 --- a/src/background/redux/sagas/actions.ts +++ b/src/background/redux/sagas/actions.ts @@ -9,7 +9,13 @@ import { SecretPhrase } from '@libs/crypto'; import { Account } from '@libs/types/account'; export const startBackground = createAction('START_BACKGROUND_SAGA'); -export const resetVault = createAction('RESET_VAULT_SAGA'); +// `senderWindowId` is attached by the background handler (`redux-actions.ts`, +// from `MessageSender`), never by the UI dispatcher — the UI's own +// `resetVault()` calls stay zero-arg. Optional: absent for a non-tab sender. +export const resetVault = createAction( + 'RESET_VAULT_SAGA', + (senderWindowId?: number) => ({ payload: { senderWindowId } }) +); export const lockVault = createAction('LOCK_VAULT_SAGA'); export const openExportKeysWindow = createAction( 'OPEN_EXPORT_KEYS_WINDOW_SAGA' diff --git a/src/background/redux/sagas/onboarding-sagas.test.ts b/src/background/redux/sagas/onboarding-sagas.test.ts index 3f2773097..7c86e7e79 100644 --- a/src/background/redux/sagas/onboarding-sagas.test.ts +++ b/src/background/redux/sagas/onboarding-sagas.test.ts @@ -5,6 +5,7 @@ import { expectSaga } from 'redux-saga-test-plan'; import { storage, windows } from 'webextension-polyfill'; import { deliverCancelResponse } from '@background/handlers/cancel-requests'; +import { sagaError } from '@background/redux/app-events/actions'; import { vaultReseted } from '@background/redux/vault/actions'; import { windowManagementReseted } from '@background/redux/windowManagement/actions'; import { reducer as windowManagementReducer } from '@background/redux/windowManagement/reducer'; @@ -88,6 +89,10 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () const openRequest = { status: 'open' as const, tabId: 3, + // Present in the fixture so a dropped-frameId pass-through fails a test: + // an omitted `frameId` resumes the unscoped broadcast + // (`deliver-via-origin`'s sub-frame refusal keys on `frameId != null`). + frameId: 5, origin: 'https://dapp', method: 'sign' as const, windowIds: [42], @@ -160,8 +165,13 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () // resets land inside the same synchronous flush as the dispatch call itself // (redux-saga drains `put`/`select` effects synchronously at semaphore 0, // before yielding back to the caller of `dispatch`). - it('lands every reset and storage.local.clear() SYNCHRONOUSLY inside store.dispatch(resetVault()) — before delivery, which never resolves, could ever run', () => { - (deliverCancelResponse as jest.Mock).mockReturnValue(new Promise(() => {})); + it('lands every reset and storage.local.clear() SYNCHRONOUSLY inside store.dispatch(resetVault()) — before window removal or the mirror clear, which never resolve, could ever run', () => { + // Resolved, not never-resolving: the CALL below is what pins the snapshot + // ordering (see the next assertion), and it happens synchronously either + // way — this only changes what the promise does afterward, which this + // test does not care about. `windows.remove` / `clearRequestSession` stay + // never-resolving; they are what proves this saga does not wait on them. + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); (windows.remove as jest.Mock).mockReturnValue(new Promise(() => {})); (clearRequestSession as jest.Mock).mockReturnValue(new Promise(() => {})); @@ -191,6 +201,15 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () exportKeysWindowId: null, requests: {} }); + // Pins that `select(selectOpenRequests)` ran BEFORE the resets: the call + // (not its resolution — that's still pending, deliberately) already + // carries the PRE-reset row. If the select ran after + // `windowManagementReseted()` instead, `openRequests` would be empty and + // this would never be called at all. + expect(deliverCancelResponse).toHaveBeenCalledWith( + expect.objectContaining({ requestId: 'r1' }), + 'resetVaultSaga' + ); }); it('delivers the cancel for an open request at reset time, from the pre-reset snapshot', async () => { @@ -201,10 +220,14 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () .dispatch(resetVault()) .silentRun(50); + // Not just a partial `objectContaining` that omits `frameId`: an + // omitted field here would still pass a check that doesn't name it, even + // though the row silently lost its frame scoping on the way through. expect(deliverCancelResponse).toHaveBeenCalledWith( expect.objectContaining({ requestId: 'r1', tabId: 3, + frameId: 5, origin: 'https://dapp', method: 'sign' }), @@ -286,7 +309,7 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () consoleError.mockRestore(); }); - it('logs, and does not throw, when windows.remove rejects', async () => { + it('logs, dispatches sagaError, and does not throw, when windows.remove rejects — while the resets still complete synchronously', async () => { (deliverCancelResponse as jest.Mock).mockResolvedValue(1); (windows.remove as jest.Mock).mockRejectedValue( new Error('no such window: https://dapp/page?message=super-secret') @@ -295,13 +318,20 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () .spyOn(console, 'error') .mockImplementation(() => {}); - await expectSaga(onboardingSagas) + const { allEffects } = await expectSaga(onboardingSagas) .withState(stateWithOneOpenRequest) .dispatch(resetVault()) .silentRun(50); await Promise.resolve(); await Promise.resolve(); + // The descriptors and the mirror are already gone by the time this fires + // — nothing else will ever find this window again, so it must not stay + // console-only. The ordering invariant (resets land before this) still + // holds: `windowManagementReseted` landed before this rejecting `put` + // could even have been effect-scheduled. + expect(countPutsOfType(allEffects, windowManagementReseted.type)).toBe(1); + expect(countPutsOfType(allEffects, sagaError.type)).toBe(1); expect(consoleError).toHaveBeenCalledWith( 'resetVaultSaga: window removal failed', { windowId: 42 }, @@ -347,4 +377,99 @@ describe('resetVaultSaga (spec §8.3 — cancel-then-clear on wallet reset)', () expect(loggedError).not.toMatch(/\?[^"]*=/); consoleError.mockRestore(); }); + + it("excludes the originating window from removal, even when it also appears in a request's windowIds", async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: { r1: { ...openRequest, windowIds: [42, 43] } } + } + }) + // 43 is the window `resetVault` was dispatched FROM — closing it would + // kill the page's own continuation. + .dispatch(resetVault(43)) + .silentRun(50); + + expect(windows.remove).toHaveBeenCalledTimes(1); + expect(windows.remove).toHaveBeenCalledWith(42); + expect(windows.remove).not.toHaveBeenCalledWith(43); + }); + + it('also removes the shared approval window and the export-keys window', async () => { + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: 7, + exportKeysWindowId: 8, + requests: {} + } + }) + .dispatch(resetVault()) + .silentRun(50); + + // Neither is a request, so `selectOpenRequests` alone would miss both: + // the shared approval window would never close, and the export-keys + // window's single-window guard would stay defeated for the rest of the + // service worker's life. + expect(windows.remove).toHaveBeenCalledTimes(2); + expect(windows.remove).toHaveBeenCalledWith(7); + expect(windows.remove).toHaveBeenCalledWith(8); + }); + + it('delivers to and removes windows for two open requests, deduping the window they share', async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + + const r1 = { ...openRequest, windowIds: [42, 43] }; + const r2 = { ...openRequest, tabId: 9, windowIds: [42] }; + + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: null, + exportKeysWindowId: null, + requests: { r1, r2 } + } + }) + .dispatch(resetVault()) + .silentRun(50); + + expect(deliverCancelResponse).toHaveBeenCalledTimes(2); + expect(windows.remove).toHaveBeenCalledTimes(2); + expect(windows.remove).toHaveBeenCalledWith(42); + expect(windows.remove).toHaveBeenCalledWith(43); + }); + + it('dedupes window removal across multiple open requests sharing a window, and covers the whole widened set at once', async () => { + (deliverCancelResponse as jest.Mock).mockResolvedValue(1); + + const r1 = { ...openRequest, windowIds: [42, 43] }; + const r2 = { ...openRequest, tabId: 9, windowIds: [42] }; + + await expectSaga(onboardingSagas) + .withState({ + windowManagement: { + windowId: 44, + exportKeysWindowId: 45, + requests: { r1, r2 } + } + }) + // 43 also names a request window (r1's) — must still be excluded. + .dispatch(resetVault(43)) + .silentRun(50); + + expect(deliverCancelResponse).toHaveBeenCalledTimes(2); + // 42 appears in both r1 and r2: removed once. 43 is the origin: excluded + // even though it names a request window. 44 (windowId) and 45 + // (exportKeysWindowId) are added from the widened snapshot. Three calls + // total — {42, 44, 45} — not four and not five. + expect(windows.remove).toHaveBeenCalledTimes(3); + expect(windows.remove).toHaveBeenCalledWith(42); + expect(windows.remove).toHaveBeenCalledWith(44); + expect(windows.remove).toHaveBeenCalledWith(45); + expect(windows.remove).not.toHaveBeenCalledWith(43); + }); }); diff --git a/src/background/redux/sagas/onboarding-sagas.ts b/src/background/redux/sagas/onboarding-sagas.ts index 3d2782a45..a914fa138 100644 --- a/src/background/redux/sagas/onboarding-sagas.ts +++ b/src/background/redux/sagas/onboarding-sagas.ts @@ -1,4 +1,4 @@ -import { put, select, takeLatest } from 'redux-saga/effects'; +import { call, fork, put, select, takeLatest } from 'redux-saga/effects'; import { storage, windows } from 'webextension-polyfill'; import { ErrorMessages } from '@src/constants'; @@ -16,7 +16,11 @@ import { recipientPublicKeyReseted } from '@background/redux/recent-recipient-pu import { vaultSettingsReseted } from '@background/redux/settings/actions'; import { resetTrustedWasmState } from '@background/redux/trusted-wasm/actions'; import { windowManagementReseted } from '@background/redux/windowManagement/actions'; -import { selectOpenRequests } from '@background/redux/windowManagement/selectors'; +import { + selectExportKeysWindowId, + selectOpenRequests, + selectWindowId +} from '@background/redux/windowManagement/selectors'; import { clearRequestSession } from '@background/redux/windowManagement/session-store'; import { OpenRequest } from '@background/redux/windowManagement/types'; import { @@ -54,13 +58,12 @@ export function* onboardingSagas() { yield takeLatest(recoverVault.type, recoverVaultSaga); } -// Fire-and-forget: delivery and window cleanup run AFTER the resets below -// have already completed, from a snapshot taken before any of them. Never -// awaited by the saga — see the ordering note there. `failRequestOnWindowError` -// cannot be reused directly here: it needs the store (to dispatch the -// tombstone), which the wallet no longer has any use for once every slice is -// already wiped, so this only needs `deliverCancelResponse`, the store-free -// half it shares. +// Fire-and-forget: delivery runs AFTER the resets below have already +// completed, from a snapshot taken before any of them. Never awaited by the +// saga — see the ordering note there. `failRequestOnWindowError` cannot be +// reused directly here: it needs the store (to dispatch the tombstone), which +// the wallet no longer has any use for once every slice is already wiped, so +// this only needs `deliverCancelResponse`, the store-free half it shares. function deliverResetCancels(openRequests: readonly OpenRequest[]): void { for (const request of openRequests) { deliverCancelResponse(request, 'resetVaultSaga').catch(error => { @@ -71,29 +74,59 @@ function deliverResetCancels(openRequests: readonly OpenRequest[]): void { ); }); } +} - const windowIds = [...new Set(openRequests.flatMap(r => r.windowIds))]; - - for (const windowId of windowIds) { - windows.remove(windowId).catch(error => { - console.error( - 'resetVaultSaga: window removal failed', - { windowId }, - redactUrlQuery(error) - ); - }); +// Forked, not a bare `.catch`: a rejection here must reach the store via +// `put`, and only a saga effect can do that from code that runs after the +// synchronous reset block (a detached Promise callback has no store to +// dispatch to). The descriptors and the mirror are already gone by the time +// this runs, so a failure here is terminal — nothing else will ever find this +// window again to retry. Window id only; no origins/URLs. +function* removeResetWindow(windowId: number) { + try { + yield call([windows, windows.remove], windowId); + } catch (error) { + console.error( + 'resetVaultSaga: window removal failed', + { windowId }, + redactUrlQuery(error) + ); + yield put( + sagaError({ + source: 'resetVaultSaga', + message: `Could not close window ${windowId} after reset` + }) + ); } } /** * */ -function* resetVaultSaga() { +function* resetVaultSaga(action: ReturnType) { try { - // Snapshotted BEFORE any reset: the reducer that clears `windowManagement` - // below throws away the descriptors this needs to cancel and to find the - // approval windows to close. + // Snapshotted BEFORE any reset: the reducers cleared below throw away the + // descriptors this needs to cancel, the approval windows to close, and the + // export-keys window id — `selectOpenRequests` alone would miss the + // latter two, which are not requests. Widened rather than left to + // `selectOpenRequests` alone: without `windowId` here the shared approval + // window is never closed, and without `exportKeysWindowId` the + // Download-account-keys window survives reset with its single-window + // guard defeated for the rest of the service worker's life (the reducer + // nulls the id but nothing closes the window it named). No key material + // is exposed by the surviving window either way — it renders the error + // page — the guard is the loss. + // + // Accepted residual: a request still between registration and + // window-attach contributes no window here (`windowIds` is still `[]`). + // Its window opens after this reset, over the now-wiped wallet, gets + // tracked via `windowIdChanged` into the fresh slice, and is reused by + // the next approval like any other — not compensated for. const openRequests: OpenRequest[] = yield select(selectOpenRequests); + const windowId: number | null = yield select(selectWindowId); + const exportKeysWindowId: number | null = yield select( + selectExportKeysWindowId + ); // Order matters and is the whole point (spec §8.3). Today the twelve // `put`s below complete synchronously inside `store.dispatch(resetVault())` @@ -133,8 +166,28 @@ function* resetVaultSaga() { // Deliveries and window removal happen strictly AFTER the synchronous // block above, from the snapshot. A slow or rejecting delivery must not // delay or break the resets or `storage.local.clear()` — it can't, since - // this call is not awaited. + // none of this is awaited. deliverResetCancels(openRequests); + + // The originating window is excluded: `ResetVaultPage` renders inside the + // signature-request and connect-to-app approval windows (`LockedRouter`), + // so removing it here would kill the page's OWN continuation + // (`closeWindowByReloadExtension`) before it runs — and on Firefox/Safari + // that also skips `runtime.reload()`. It converges on its own instead: + // Chrome closes itself via `window.close()`, Firefox/Safari die with + // `runtime.reload()`. + const senderWindowId = action.payload.senderWindowId; + const windowIdsToRemove = new Set( + [ + ...openRequests.flatMap(r => r.windowIds), + windowId, + exportKeysWindowId + ].filter((id): id is number => id != null && id !== senderWindowId) + ); + + for (const id of windowIdsToRemove) { + yield fork(removeResetWindow, id); + } } catch (err) { console.error(err); yield put(