fix: gate the chain switch and remember endpoints per network (closes #308)

wallet_switchEthereumChain was answered for any origin at all, with no
connection check and no prompt, so a page the user had never connected to
could move the active chain — clearing the [TESTNET] banner under someone
who believed they were on Sepolia. It now takes the same
allowedSites/connectedSites gate the signing methods take, ahead of the
same-chain and unsupported-chain answers, and refuses an unconnected origin
with 4100.

The switch also overwrote state.rpcUrl and state.blockscoutUrl with the
network defaults, so a user running their own node lost that url
permanently and silently to a public endpoint that then sees every address
they hold. Endpoints are now remembered per network in
state.networkEndpoints: the switch snapshots the network being left and
restores the network being entered, falling back to that network's
defaults. state.rpcUrl and state.blockscoutUrl remain the live endpoints of
the active network, so no reader changed; for the active network they are
authoritative and the map entry may be stale, and the snapshot is what
reconciles them. A profile written before the map existed has its stored
pair adopted for the network it was stored under, so nothing is lost on
first load.

TODO.md

@@ -60,12 +60,7 @@ but the review is broader than any of them.
   for the network being entered; `state.rpcUrl` stays the live value for the
   active network, so no reader changed. A profile written before the map existed
   has its stored pair adopted for the network it was stored under, and loses
-  nothing. The handler now loads state before it switches
+  nothing.
-  ([#316](https://git.eeqj.de/sneak/AutistMask/issues/316)): the service worker
-  populates nothing at module scope, so a worker revived by the page's own
-  message held `DEFAULT_STATE`, and the switch persisted every field of it —
-  wiping every wallet, every site approval and every tracked token from storage
-  along with the endpoint.
 - 2026-08-17: The Settings screen is driven in a browser, and every element id
   the popup looks up is checked statically. Nothing exercised Settings in the
   e2e suite, and jest runs with no DOM, so the densest run of `$("...")` lookups

src/background/index.js

@@ -689,17 +689,6 @@ async function handleRpc(method, params, origin) {
             return { error: { code: 4100, message: "Unauthorized" } };
         }
 
-        // onChainSwitch() mutates the module-level state singleton and then
-        // saves every field of it, and currentNetwork() reads the same
-        // singleton. This worker may have been started by this very message:
-        // nothing loads state at module scope, so without this the singleton
-        // is DEFAULT_STATE, the same-chain check compares against the wrong
-        // network, and the save writes empty wallets, empty allowedSites and
-        // the default endpoints over the user's stored profile
-        // (https://git.eeqj.de/sneak/AutistMask/issues/316). Same precedent
-        // as the transaction path below.
-        await loadState();
-
         const chainId = params?.[0]?.chainId;
         if (chainId === currentNetwork().chainId) {
             return { result: null };

src/shared/state.js

@@ -137,18 +137,8 @@ async function loadState() {
         state.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl;
         state.blockscoutUrl =
             saved.blockscoutUrl || DEFAULT_STATE.blockscoutUrl;
-        // An actual object is required, not merely a truthy non-array: the
-        // code below and onChainSwitch() index and ASSIGN INTO this value,
-        // and assigning a property to a string or a number is a silent no-op
-        // in sloppy mode. A stored primitive would therefore be re-persisted
-        // unchanged forever, and every switch would fall back to the network
-        // default — the endpoint loss this map exists to prevent, with no
-        // self-healing. The allowedSites/deniedSites guards below are only
-        // read from, which is why they can be looser.
         state.networkEndpoints =
-            typeof saved.networkEndpoints === "object" &&
+            saved.networkEndpoints && !Array.isArray(saved.networkEndpoints)
-            saved.networkEndpoints !== null &&
-            !Array.isArray(saved.networkEndpoints)
                 ? saved.networkEndpoints
                 : {};
         // A profile written before this map existed carries exactly one pair

tests/coldWorkerChainSwitch.test.js

@@ -1,212 +0,0 @@
-// What a chain switch does to 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. The chain-switch handler
-// reaches onChainSwitch(), which mutates the module-level `state` singleton
-// and then persists EVERY field of it, so a handler that runs before a load
-// writes DEFAULT_STATE over the user's stored profile — every wallet, every
-// site approval, every tracked token and the custom endpoint
-// (https://git.eeqj.de/sneak/AutistMask/issues/316). The same singleton is
-// what currentNetwork() answers from, so the same-chain early return also
-// compares against the wrong network.
-//
-// This file therefore uses the REAL state module and never calls loadState()
-// itself: the handler has to do it. tests/chainSwitchGate.test.js mocks the
-// state module wholesale and tests/networkEndpoints.test.js always loads
-// first, so neither can see this.
-
-const { networkById } = require("../src/shared/networks");
-
-const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
-
-const CONNECTED_ORIGIN = "https://dapp.example";
-const CONNECTED_HOSTNAME = "dapp.example";
-
-const MAINNET = networkById("mainnet");
-const SEPOLIA = networkById("sepolia");
-
-// The user's own node, and a wallet whose loss is the whole point.
-const CUSTOM_RPC = "http://127.0.0.1:8545";
-const CUSTOM_BLOCKSCOUT = "http://127.0.0.1:4000/api/v2";
-const TOKEN = "0x6B175474E89094C44Da98b954EedeAC495271d0F";
-
-function walletFixture() {
-    return [
-        {
-            name: "Wallet 1",
-            type: "hd",
-            addresses: [{ address: ADDRESS, balance: "0", tokenBalances: [] }],
-        },
-    ];
-}
-
-// A profile as an installed extension holds it, on `networkId`.
-function storedProfile(networkId) {
-    return {
-        hasWallet: true,
-        wallets: walletFixture(),
-        activeAddress: ADDRESS,
-        networkId,
-        rpcUrl: CUSTOM_RPC,
-        blockscoutUrl: CUSTOM_BLOCKSCOUT,
-        allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
-        deniedSites: {},
-        trackedTokens: [{ address: TOKEN, symbol: "DAI", decimals: 18 }],
-        theme: "dark",
-    };
-}
-
-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 and chain-switch modules
-// behind it, over a storage stub that actually keeps what is written — a
-// wipe is only observable against storage that remembers.
-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 toTabs = [];
-
-    global.chrome = {
-        storage: {
-            local: {
-                get: jest.fn(async () => ({ autistmask: store.autistmask })),
-                set: jest.fn(async (items) => {
-                    store.autistmask = items.autistmask;
-                }),
-            },
-        },
-        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) => {
-                toTabs.push(message);
-                if (cb) cb();
-            },
-        },
-        action: { setPopup: () => {} },
-    };
-
-    require("../src/background/index");
-
-    async function switchChain(chainId) {
-        let result = null;
-        messageListener(
-            {
-                type: "AUTISTMASK_RPC",
-                method: "wallet_switchEthereumChain",
-                params: [{ chainId }],
-            },
-            { origin: CONNECTED_ORIGIN },
-            (r) => {
-                result = r;
-            },
-        );
-        await settle();
-        return result;
-    }
-
-    return {
-        switchChain,
-        persisted: () => store.autistmask,
-        chainChangedEvents: () =>
-            toTabs.filter((m) => m.eventName === "chainChanged"),
-    };
-}
-
-describe("a chain switch on a worker that never loaded state", () => {
-    test("keeps the wallets, approvals, tokens and custom endpoint", async () => {
-        const bg = loadColdWorker("mainnet");
-
-        const result = await bg.switchChain(SEPOLIA.chainId);
-        expect(result).toEqual({ result: null });
-
-        const after = bg.persisted();
-        // The switch itself happened.
-        expect(after.networkId).toBe("sepolia");
-        expect(after.rpcUrl).toBe(SEPOLIA.defaultRpcUrl);
-
-        // And it took nothing else with it. Without the load these come back
-        // as [], {}, [] and "system" from DEFAULT_STATE — every wallet in the
-        // extension gone, encrypted secrets included.
-        expect(after.wallets).toEqual(walletFixture());
-        expect(after.hasWallet).toBe(true);
-        expect(after.activeAddress).toBe(ADDRESS);
-        expect(after.allowedSites).toEqual({ [ADDRESS]: [CONNECTED_HOSTNAME] });
-        expect(after.trackedTokens).toEqual([
-            { address: TOKEN, symbol: "DAI", decimals: 18 },
-        ]);
-        expect(after.theme).toBe("dark");
-
-        // The user's mainnet endpoint is remembered rather than replaced by
-        // the public default, so switching back returns it.
-        expect(after.networkEndpoints.mainnet).toEqual({
-            rpcUrl: CUSTOM_RPC,
-            blockscoutUrl: CUSTOM_BLOCKSCOUT,
-        });
-
-        await bg.switchChain(MAINNET.chainId);
-        expect(bg.persisted().rpcUrl).toBe(CUSTOM_RPC);
-        expect(bg.persisted().blockscoutUrl).toBe(CUSTOM_BLOCKSCOUT);
-        expect(bg.persisted().wallets).toEqual(walletFixture());
-    });
-
-    test("compares the requested chain against the stored one, not the default", async () => {
-        // Stored on Sepolia, asked for mainnet. Reading the unloaded
-        // singleton makes this look like the chain already active, so the
-        // page is told the switch succeeded while the wallet stays on the
-        // testnet it was on.
-        const bg = loadColdWorker("sepolia");
-
-        const result = await bg.switchChain(MAINNET.chainId);
-        expect(result).toEqual({ result: null });
-
-        expect(bg.persisted().networkId).toBe("mainnet");
-        expect(bg.chainChangedEvents()).toEqual([
-            {
-                type: "AUTISTMASK_EVENT",
-                eventName: "chainChanged",
-                data: MAINNET.chainId,
-            },
-        ]);
-    });
-});

tests/networkEndpoints.test.js

@@ -154,22 +154,11 @@ describe("a custom endpoint survives a chain switch", () => {
         expect(mod.state.blockscoutUrl).toBe(CUSTOM_BLOCKSCOUT);
     });
 
-    // A primitive is the dangerous case, not the array: assigning a property
+    test("a stored networkEndpoints of the wrong type is discarded", async () => {
-    // to a string throws nothing and stores nothing, so a stored string would
+        const { mod } = loadModuleWith({
-    // be carried through loadState() and re-persisted by every save, and each
-    // switch would fall back to the public default in place of the user's
-    // endpoint, permanently.
-    test.each([
-        ["an array", ["not", "a", "map"]],
-        ["a string", "junk"],
-        ["a number", 7],
-    ])("a stored networkEndpoints that is %s is discarded", async (_, bad) => {
-        const { mod, chainSwitch } = loadModuleWith({
             wallets: walletFixture(),
             networkId: "mainnet",
-            rpcUrl: CUSTOM_RPC,
+            networkEndpoints: ["not", "a", "map"],
-            blockscoutUrl: CUSTOM_BLOCKSCOUT,
-            networkEndpoints: bad,
         });
         await mod.loadState();
 
@@ -177,16 +166,9 @@ describe("a custom endpoint survives a chain switch", () => {
         // profile is — never left as something onChainSwitch() would index.
         expect(mod.state.networkEndpoints).toEqual({
             mainnet: {
-                rpcUrl: CUSTOM_RPC,
+                rpcUrl: MAINNET.defaultRpcUrl,
-                blockscoutUrl: CUSTOM_BLOCKSCOUT,
+                blockscoutUrl: MAINNET.defaultBlockscoutUrl,
             },
         });
-
-        // And the endpoint really survives the round trip, which is the point
-        // of discarding it rather than only of the shape being right.
-        await chainSwitch.onChainSwitch("sepolia");
-        await chainSwitch.onChainSwitch("mainnet");
-        expect(mod.state.rpcUrl).toBe(CUSTOM_RPC);
-        expect(mod.state.blockscoutUrl).toBe(CUSTOM_BLOCKSCOUT);
     });
 });