diff --git a/TODO.md b/TODO.md index fd5e7dd..93a271b 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,21 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-20: A page asking which chain the wallet is on is told the chain the + user is actually on ([#317](https://git.eeqj.de/sneak/AutistMask/issues/317)). + `eth_chainId` and `net_version` answered from `currentNetwork()`, which reads + the module-level `state` singleton that nothing populates at module scope, so + a service worker revived by the page's own message answered out of + `DEFAULT_STATE` and reported mainnet `0x1`/`1` to a user on Sepolia — a dApp + building its interaction for the wrong chain. Both now `await loadState()` + first, under one load covering the pair. The read side of the background was + audited with it: the remaining singleton reads are the chain switch, the + transaction verification path and `backgroundRefresh`, which each already + load, and everything else answers from storage per call through `getState()`. + One stale read is left named but unfixed, outside this issue's scope: + `handleSendTransaction` builds its provider with no network name, so + `getProvider()` falls back to the same unloaded singleton for ethers' static + network hint. - 2026-08-20: A web page can no longer switch the wallet's chain, and switching no longer destroys the user's endpoints ([#308](https://git.eeqj.de/sneak/AutistMask/issues/308)). diff --git a/src/background/index.js b/src/background/index.js index 58c349e..c364338 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -663,12 +663,21 @@ async function handleRpc(method, params, origin) { return { result: [] }; } - if (method === "eth_chainId") { - return { result: currentNetwork().chainId }; - } - - if (method === "net_version") { - return { result: currentNetwork().networkVersion }; + // Both answer from currentNetwork(), which reads the module-level state + // singleton, and nothing populates that at module scope. A worker revived + // by the page's own message therefore held DEFAULT_STATE and told a page + // it was on mainnet while the user was on Sepolia + // (https://git.eeqj.de/sneak/AutistMask/issues/317). One load covers both: + // they are the same read of the same value, and the switch handler below + // and the transaction path have the same await for the same reason. + if (method === "eth_chainId" || method === "net_version") { + await loadState(); + return { + result: + method === "eth_chainId" + ? currentNetwork().chainId + : currentNetwork().networkVersion, + }; } if (method === "wallet_switchEthereumChain") { diff --git a/tests/coldWorkerChainId.test.js b/tests/coldWorkerChainId.test.js new file mode 100644 index 0000000..12b46bd --- /dev/null +++ b/tests/coldWorkerChainId.test.js @@ -0,0 +1,192 @@ +// What eth_chainId and net_version answer on a worker that has not loaded +// state yet. +// +// The MV3 service worker is terminated when idle and revived by the next +// message, and nothing loads state at module scope. Both methods answer from +// currentNetwork(), which reads the module-level `state` singleton, so a +// worker revived by the page's own message answered out of DEFAULT_STATE and +// told a page it was on mainnet while the user was on Sepolia +// (https://git.eeqj.de/sneak/AutistMask/issues/317). +// +// This file therefore uses the REAL state module and never calls loadState() +// itself: the handler has to do it. Same shape as +// tests/coldWorkerChainSwitch.test.js, which covers the write side. + +const { networkById } = require("../src/shared/networks"); + +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +const CONNECTED_ORIGIN = "https://dapp.example"; +const CONNECTED_HOSTNAME = "dapp.example"; +const UNKNOWN_ORIGIN = "https://stranger.example"; + +const MAINNET = networkById("mainnet"); +const SEPOLIA = networkById("sepolia"); + +function storedProfile(networkId) { + return { + hasWallet: true, + wallets: [ + { + name: "Wallet 1", + type: "hd", + addresses: [ + { address: ADDRESS, balance: "0", tokenBalances: [] }, + ], + }, + ], + activeAddress: ADDRESS, + networkId, + rpcUrl: networkById(networkId).defaultRpcUrl, + blockscoutUrl: networkById(networkId).defaultBlockscoutUrl, + allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] }, + deniedSites: {}, + trackedTokens: [], + }; +} + +async function settle() { + for (let i = 0; i < 50; i++) await Promise.resolve(); +} + +afterEach(() => { + delete global.chrome; +}); + +// Load the background worker with the real state module behind it, over a +// storage stub that keeps what is written — so the test can also show that +// answering a read method persists nothing. +function loadColdWorker(networkId) { + jest.resetModules(); + + jest.doMock("../src/shared/balances", () => ({ + getProvider: () => ({}), + refreshBalances: jest.fn(async () => {}), + })); + jest.doMock("../src/shared/phishingDomains", () => ({ + isPhishingDomain: () => false, + })); + jest.doMock("../src/shared/alarms", () => ({ + BALANCE_REFRESH_ALARM: "balance", + BALANCE_REFRESH_PERIOD_MINUTES: 1, + ensureRecurringAlarms: jest.fn(async () => {}), + registerAlarmHandlers: jest.fn(), + })); + + const store = { autistmask: storedProfile(networkId) }; + + let messageListener = null; + const set = jest.fn(async (items) => { + store.autistmask = items.autistmask; + }); + + global.chrome = { + storage: { + local: { + get: jest.fn(async () => ({ autistmask: store.autistmask })), + set, + }, + }, + runtime: { + getURL: (path) => "chrome-extension://autistmask/" + path, + onMessage: { + addListener: (fn) => { + messageListener = fn; + }, + }, + onConnect: { addListener: () => {} }, + lastError: null, + }, + windows: { + getLastFocused: (cb) => cb(null), + create: (options, cb) => cb({ id: 1 }), + remove: (id, cb) => { + if (cb) cb(); + }, + onRemoved: { addListener: () => {} }, + }, + tabs: { + query: (queryInfo, cb) => cb([{ id: 1 }]), + sendMessage: (tabId, message, cb) => { + if (cb) cb(); + }, + }, + action: { setPopup: () => {} }, + }; + + require("../src/background/index"); + + async function rpc(method, origin) { + let result = null; + messageListener( + { type: "AUTISTMASK_RPC", method, params: [] }, + { origin: origin || CONNECTED_ORIGIN }, + (r) => { + result = r; + }, + ); + await settle(); + return result; + } + + return { rpc, persisted: () => store.autistmask, storageSet: set }; +} + +describe("chain identity read by a worker that never loaded state", () => { + test("eth_chainId answers the stored chain, not the default", async () => { + // The first message this worker ever sees. Reading the unloaded + // singleton answers mainnet's 0x1 to a user who is on Sepolia. + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("eth_chainId")).toEqual({ + result: SEPOLIA.chainId, + }); + }); + + test("net_version answers the stored chain, not the default", async () => { + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("net_version")).toEqual({ + result: SEPOLIA.networkVersion, + }); + }); + + test("answers the stored chain to an origin that never connected", async () => { + // Neither method is gated on a connection, so the stale answer reached + // any page at all; the fixed answer has to as well. + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("eth_chainId", UNKNOWN_ORIGIN)).toEqual({ + result: SEPOLIA.chainId, + }); + expect(await bg.rpc("net_version", UNKNOWN_ORIGIN)).toEqual({ + result: SEPOLIA.networkVersion, + }); + }); + + test("answers mainnet for a profile stored on mainnet", async () => { + // The default and the stored value agree here, so this case cannot + // catch the defect; it is what keeps the fix from being a swap. + const bg = loadColdWorker("mainnet"); + + expect(await bg.rpc("eth_chainId")).toEqual({ + result: MAINNET.chainId, + }); + expect(await bg.rpc("net_version")).toEqual({ + result: MAINNET.networkVersion, + }); + }); + + test("persists nothing: these are reads", async () => { + // The load must not turn a read into a write. saveState() persists + // every field of the singleton, and a read path that reached it would + // be the wipe https://git.eeqj.de/sneak/AutistMask/issues/316 fixed. + const bg = loadColdWorker("sepolia"); + + await bg.rpc("eth_chainId"); + await bg.rpc("net_version"); + + expect(bg.storageSet).not.toHaveBeenCalled(); + expect(bg.persisted()).toEqual(storedProfile("sepolia")); + }); +});