feat: vendor and censor the phishing blocklist at build time (closes #219)

README.md

@@ -103,12 +103,12 @@ provide:
   fmt-check
 - `script/check-censored` — assert the competitor name RULES.md bars appears
   nowhere in the working tree or under `dist/` outside its documented
-  exceptions, which are the pinned source reference in `script/vendor-blocklist`
+  exceptions: the pinned source reference in `script/vendor-blocklist`, the two
-  and the two literals shipped code cannot avoid (the `isMetaMask`/`_metamask`
+  provider-shim identifiers in `src/content/inpage.js`, and one ERC-20's
-  provider shim, and one ERC-20's on-chain name in the token list). Part of
+  on-chain name in `src/shared/tokenList.js`. Each is scoped to that path and
-  `make check`, which inspects `dist/` when there is one and says loudly when
+  fails anywhere else. Part of `make check`, which inspects `dist/` when there
-  there is not; `make build` re-runs it with `--require-dist`, so a build
+  is one and says loudly when there is not; `make build` re-runs it with
-  artifact is always covered
+  `--require-dist`, so a build artifact is always covered
 - `script/vendor-blocklist` — refresh `src/shared/phishingBlocklist.json` from
   its upstream, pinned to a commit and to the sha256 of the bytes that commit
   serves. Run deliberately, never as part of a build: the output is committed
@@ -180,6 +180,23 @@ fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
 offline; unrecognised outbound requests are reported as failures rather than
 silently allowed.
 
+It also covers the **Settings screen**, which holds the densest run of element
+id lookups in the codebase and where one wrong id leaves the whole popup blank
+rather than only degrading Settings: that the screen renders populated — the
+About well and the wallet list are read back, so a `show()` that stopped early
+is caught rather than merely a view that failed to appear — that the four Token
+Spam Protection controls are real checkboxes defaulted on, and that the theme
+and network selectors offer exactly the choices `src/shared/networks.js` and
+`index.html` define. What the selectors persist is then driven through the UI to
+`dark` and `sepolia` and reasserted after a popup reopen before being restored
+the same way; neither value is its selector's first `<option>`, so neither can
+be read back from the markup with no JavaScript having run. One spam filter is
+likewise toggled off and back on across a reopen each way, which exercises the
+change handler, `saveState()`, `loadState()` and the assignment `init()` makes.
+Each group of these assertions records a coverage key and a final case demands
+the exact set, so a section that silently stopped running reddens the suite
+instead of quietly shrinking it.
+
 It also covers the confirmation screen, for both a native ETH send and an ERC-20
 send: Send disabled while the fee estimate is in flight, enabled once it lands,
 the fee block quoting the expected cost and the reserve separately, the distinct
@@ -415,6 +432,24 @@ the one-time pull of the pinned ~800MB Playwright layer, and well under a minute
 once that layer is cached. `e2e-firefox` about 1m05s cold, and it caches its
 Firefox and geckodriver downloads the same way.
 
+### Element id guard (part of `make check`)
+
+`tests/popupElementIds.test.js` asserts statically that every element id the
+popup looks up — `$("...")`, `document.getElementById("...")`,
+`showError()`/`hideError()`, and the `view-<name>` a literal `showView("...")`
+resolves to — exists in `src/popup/index.html`, and that `index.html` defines no
+id twice. A wrong id is valid JavaScript naming a defined function, so neither
+jest (node environment, no DOM) nor a linter objects to it; at runtime `$()`
+returns `null` and the next property access throws, which inside a view's
+`init()` aborts the rest of `src/popup/index.js` `init()` and leaves the popup
+blank.
+
+It runs with no browser, so unlike the e2e suites it fits inside `make check`,
+and it covers every view rather than the ones some test happens to open. It only
+sees literal arguments: a call like `$(containerId)` is invisible to it, and a
+lookup naming the wrong existing element is valid by construction. Both of those
+are the browser suites' job.
+
 ## Rationale
 
 Common popular EVM wallets have become bloated with swap UIs, portfolio
@@ -1772,12 +1807,14 @@ The full license texts for these third-party files are included in the
 [LICENSE](LICENSE) file. The `eth-phishing-detect` row carries no repository
 link because the upstream is hosted under a competitor's organization name,
 which project policy keeps out of code and documentation.
-`script/vendor-blocklist` is the single exception and the single definition: it
+`script/vendor-blocklist` is the single definition and the only file that spells
-is build-time tooling, never shipped, and it records the exact URL, the commit
+the name in prose: it is build-time tooling, never shipped, and it records the
-it is pinned to and the sha256 of the bytes that commit serves, because a source
+exact URL, the commit it is pinned to and the sha256 of the bytes that commit
-reference nobody can verify is not a source reference. `script/check-censored`
+serves, because a source reference nobody can verify is not a source reference.
-reads the name back out of that one file and fails the build if it appears
+`script/check-censored` reads the name back out of that one file and fails the
-anywhere else.
+build wherever else it appears, save for three shipped-code literals it cannot
+avoid — each permitted only at the one path that carries it, and listed in that
+script's header.
 
 ## Author
 

TODO.md

@@ -44,6 +44,34 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 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
+  in the codebase was unverified at runtime. Seven new cases in
+  `tests/e2e/run.js` reach Settings, assert the About well and the wallet list
+  were actually written, assert the four Token Spam Protection checkboxes are
+  real checkboxes defaulted on, and assert the theme and network selectors offer
+  the choices `src/shared/networks.js` and `index.html` define. The selectors
+  are then driven to `dark` and `sepolia` — neither is the first `<option>`, so
+  neither can be read back from the markup with no JavaScript involved — and
+  reasserted after a popup reopen before being restored the same way, and one
+  spam filter is toggled off and back on across a reopen each way. Those round
+  trips run the change handler, `saveState()`, `loadState()` and the
+  `init()`/`show()` assignments rather than just looking at the screen. `show()`
+  no longer guards its `settings-network` lookup with `if (networkSelect)`: a
+  missing element must fail loudly, which is the whole failure mode this unit
+  exists to catch. Each group records a coverage key and a final case demands
+  the exact set, so a shortened or skipped section reddens the run instead of
+  shrinking it. `tests/popupElementIds.test.js` is the general half and runs in
+  `make check` with no browser: every literal id reached through `$()`,
+  `document.getElementById()`, `showError()`/`hideError()` and `showView()` must
+  exist in `src/popup/index.html`, which no id in `index.html` may define twice.
+  Demonstrated on four deliberate breaks — a typo'd id (both halves red), a
+  handler bound to the wrong but existing element (only the functional e2e case
+  red), a typo in a view no browser suite opens (only the static guard red), and
+  the deletion of both persisted-value assignments in `settings.js` (only the
+  selector round-trip case red)
+  ([#229](https://git.eeqj.de/sneak/AutistMask/issues/229)).
 - 2026-08-17: The phishing blocklist is vendored at build time and censored, and
   the runtime fetch is gone
   ([#219](https://git.eeqj.de/sneak/AutistMask/issues/219)).
@@ -53,12 +81,13 @@ but the review is broader than any of them.
   domain names — which is what removes the competitor's name from a list that
   carried it 6,475 times, without dropping a single one of those domains.
   `script/check-censored` runs in `make check` and again against `dist/` at the
-  end of every build; the name now appears only in the vendoring script, which
+  end of every build, each permitted occurrence scoped to the one path allowed
-  defines it once, and in the two literals shipped code cannot avoid (the
+  to carry it; the name now appears only in the vendoring script, which defines
-  `isMetaMask`/`_metamask` provider shim, and one ERC-20's on-chain name).
+  it once, in the provider-shim identifiers in `src/content/inpage.js`, and in
-  Removing the fetch retired the delta, the persistence and the 24-hour alarm
+  one ERC-20's on-chain name in `src/shared/tokenList.js`. Removing the fetch
-  from [#158](https://git.eeqj.de/sneak/AutistMask/issues/158), and retired
+  retired the delta, the persistence and the 24-hour alarm from
-  alarms are now cleared rather than left running on existing installs. Two
+  [#158](https://git.eeqj.de/sneak/AutistMask/issues/158), and retired alarms
+  are now cleared rather than left running on existing installs. Two
   consequences, both deliberate: the list no longer self-updates, so it is as
   fresh as the last vendoring run that was released; and re-vendoring from
   current upstream took it from 231,357 stale entries to 105,721 current ones,
@@ -199,6 +228,39 @@ but the review is broader than any of them.
   under load, filed as [#287](https://git.eeqj.de/sneak/AutistMask/issues/287)
   rather than papered over
   ([#259](https://git.eeqj.de/sneak/AutistMask/issues/259)).
+- 2026-08-14: A background message handler that throws now rejects the page
+  instead of hanging it. `handleRpc(...).then(sendResponse)` had no `.catch()`,
+  and `sendResponse` is the only thing that settles the dApp's
+  `window.ethereum.request()` promise — so any throw inside `handleRpc` left
+  that promise pending forever, with no error and no timeout, indistinguishable
+  from a slow wallet. It now answers `{ code: -32603, message }` (the JSON-RPC
+  internal error EIP-1474 defines and EIP-1193 defers to; no EIP-1193 4xxx code
+  describes "the wallet broke" and none was invented) and logs the method and
+  the throw to the background console rather than swallowing them. The two async
+  IIFEs behind `AUTISTMASK_TX_RESPONSE` and `AUTISTMASK_SIGN_RESPONSE` were the
+  same shape one level down — every statement inside a `try`, but a throw out of
+  a `catch` block escaping unhandled — and each got a last-resort `.catch()`
+  settling the approval through `settleApproval()` and answering the popup. The
+  transaction one tracks which phase it escaped from and reports that, so an
+  escape before `broadcastTransaction()` says the request is gone rather than
+  that it may still have reached the network. Every other handler on the path is
+  synchronous. All four are driven by real failures — a rejecting storage read,
+  and a failure classifier that throws while classifying a genuine verification
+  or broadcast failure — and were demonstrated failing first, the RPC one with
+  `sendResponse` at zero calls
+  ([#280](https://git.eeqj.de/sneak/AutistMask/issues/280)).
+- 2026-08-14: Approving a site connection is no longer a race against the popup
+  closing. The decision now rides the approval port the popup already holds,
+  which is the same channel the close disconnects, so it is delivered ahead of
+  that disconnect however fast the teardown is; `windows.onRemoved` no longer
+  decides a site approval whose port is connected, since that event is ordered
+  against nothing either. Rejecting and closing without deciding both still
+  report a rejection, and the popup delays its own close by nothing. The e2e
+  harness's deferred-`window.close()` accommodation is gone with it, so the two
+  site-prompt tests now drive the shipped decide-then-close in a real Chromium;
+  against the unfixed code the approval came back to the page as
+  `{"settled":"rejected","code":4001}`
+  ([#275](https://git.eeqj.de/sneak/AutistMask/issues/275)).
 - 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js`
   rebuilt every failure as `new Error(error.message)`, so the code the
   background produced and the content script relayed intact was dropped in the

script/check-censored

@@ -9,25 +9,27 @@
 #   - script/vendor-blocklist. Build-time tooling, never shipped. A pinned
 #     source reference that does not say what the source is cannot be verified
 #     by anyone, so it names it. Whole-file exemption.
-#   - the literal `isMetaMask` and the `_metamask` object in
+#   - the two provider-shim identifiers in src/content/inpage.js. Protocol
-#     src/content/inpage.js. A protocol identifier dApps feature-detect on;
+#     identifiers dApps feature-detect on; renaming them does not rename them in
-#     renaming it does not rename it in their code, it only stops this wallet
+#     their code, it only stops this wallet working on their sites.
-#     working on their sites.
+#   - the on-chain name of the MUSD ERC-20 in src/shared/tokenList.js. It is not
-#   - the literal `MetaMask USD` in src/shared/tokenList.js. The on-chain name
+#     what backs symbol-spoof detection — that reads symbol and address — but
-#     of an ERC-20 the user may hold, recorded next to its address and symbol,
+#     the wallet already surfaces the on-chain name of any token the user holds
-#     which is what lets symbol-spoofing detection tell the real one from a
+#     (src/shared/balances.js), and this contract's on-chain name is that
-#     forgery. Altering it would misidentify an asset to its owner.
+#     string, so censoring the repo cannot stop the wallet displaying it.
+#     Dropping the entry instead would cost the user MUSD spoof detection.
 #
 # Everything else fails, in the working tree and under dist/. The last two are
-# literals rather than files, so they are enforced by counting: a file may
+# literals rather than whole files, so they are enforced by counting, and each
-# contain the name only as many times as it contains those exact strings. The
+# literal is scoped to the path allowed to carry it: a file may contain the name
-# emitted bundles carry both, so a plain "the name must not appear in dist/"
+# only as many times as it contains the literals permitted *there*, and zero
-# could never have passed.
+# times anywhere else. The emitted bundles carry them too, so a plain "the name
+# must not appear in dist/" could never have passed.
 #
-# The name itself is not written here. script/vendor-blocklist is the one place
+# The name itself is not written in this file. script/vendor-blocklist is the
-# in this repo that defines it, and this reads it back out of there — so the
+# one place in this repo that defines it, and this reads it back out of there —
-# repo-wide grep this check exists to enforce keeps returning exactly the files
+# so the repo-wide grep this check exists to enforce keeps returning exactly the
-# named above, and this file is not one of them.
+# files named above, and this file is not one of them.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -42,7 +44,7 @@ SCAN_FLAG="--scan-paths"
 
 VENDOR_SCRIPT="$ROOT/script/vendor-blocklist"
 
-# Set by extract_name / write_allowed_literals.
+# Set by extract_name / make_literals_file.
 NAME=""
 ALLOWED_LITERALS_FILE=""
 
@@ -79,17 +81,32 @@ extract_name() {
     esac
 }
 
-# Occurrences allowed anywhere, including in the emitted bundles. Each contains
+make_literals_file() {
-# the name exactly once, which is what makes counting them sound.
-write_allowed_literals() {
     ALLOWED_LITERALS_FILE="$(mktemp \
         "${TMPDIR:-/tmp}/autistmask-censored.XXXXXX")" ||
         fail "could not create a temporary file, so nothing was scanned."
-    {
+}
-        echo "is$NAME"
+
-        echo "_$NAME"
+# The literals $1 may carry, and nothing else may. Each contains the name
-        echo "$NAME USD"
+# exactly once, which is what makes counting them sound; each is scoped to its
-    } >"$ALLOWED_LITERALS_FILE"
+# path, so a file with no business carrying the name fails even when it spells
+# it the way shipped code has to. Scoping is the point: permitting these
+# literals in any file is what once let this check pass its own prose.
+#
+# The emitted paths are listed next to the sources they come from. If the
+# bundler moves one, this goes red and the new path gets added deliberately,
+# rather than a wildcard over dist/ covering whatever lands there.
+allowed_literals_for() {
+    : >"$ALLOWED_LITERALS_FILE"
+    case "$1" in
+    src/content/inpage.js | dist/*/src/content/inpage.js)
+        printf 'is%s\n_%s\n' "$NAME" "$NAME" >"$ALLOWED_LITERALS_FILE"
+        ;;
+    src/shared/tokenList.js | dist/*/src/background/index.js | \
+        dist/*/src/popup/index.js)
+        printf '%s USD\n' "$NAME" >"$ALLOWED_LITERALS_FILE"
+        ;;
+    esac
 }
 
 # How many times does $1 contain the name (TOTAL), and how many of those are one
@@ -114,6 +131,14 @@ count_matches() {
         return 0
     fi
 
+    # No literal is permitted at this path, so every occurrence is a violation.
+    # Handled here rather than by grep, which is not required to say anything
+    # useful about an empty pattern file.
+    if [ ! -s "$ALLOWED_LITERALS_FILE" ]; then
+        ALLOWED=0
+        return 0
+    fi
+
     _cm_status=0
     _cm_out="$(grep -a -o -i -F -f "$ALLOWED_LITERALS_FILE" -- "$1")" ||
         _cm_status=$?
@@ -131,17 +156,26 @@ count_matches() {
 # counting as everything else rather than a second copy of it.
 scan_paths() {
     for _file in "$@"; do
-        case "$_file" in
+        # dist/ arrives absolute (find) and the worktree relative (git
-        "$VENDOR_SCRIPT" | "script/vendor-blocklist") continue ;;
+        # ls-files). The allowlist is keyed on repo-relative paths, so both
+        # forms are reduced to one before anything is decided about them.
+        _rel="$_file"
+        case "$_rel" in
+        "$ROOT"/*) _rel="${_rel#"$ROOT"/}" ;;
+        esac
+
+        case "$_rel" in
+        script/vendor-blocklist) continue ;;
         esac
         [ -f "$_file" ] || continue
 
+        allowed_literals_for "$_rel"
         count_matches "$_file"
         [ "$TOTAL" -gt "$ALLOWED" ] || continue
 
         FAILED=$((FAILED + 1))
-        echo "check-censored: $_file: $TOTAL occurrence(s) of the name," \
+        echo "check-censored: $_rel: $TOTAL occurrence(s) of the name," \
-            "$ALLOWED of them allowed" >&2
+            "$ALLOWED of them allowed at this path" >&2
         grep -a -n -i -F -e "$NAME" -- "$_file" | cut -c1-140 | head -5 >&2
     done
     [ "$FAILED" -eq 0 ]
@@ -210,7 +244,7 @@ main() {
     if [ "${1-}" = "$SCAN_FLAG" ]; then
         shift
         extract_name
-        write_allowed_literals
+        make_literals_file
         scan_paths "$@"
         return $?
     fi
@@ -223,7 +257,7 @@ main() {
     esac
 
     extract_name
-    write_allowed_literals
+    make_literals_file
 
     echo "Checking for censored names..."
 

src/background/index.js

@@ -165,6 +165,16 @@ function approvedNonce(approvedTx) {
     }
 }
 
+// What the page is told when a request failed in a way the wallet has no
+// specific answer for. -32603 is the JSON-RPC internal error EIP-1474 defines
+// and EIP-1193 defers to for RPC-layer failures; no EIP-1193 4xxx code
+// describes "the wallet broke", and one is not invented here. The cause is
+// logged rather than put in the message: the page gets a stable sentence, the
+// background console gets the throw.
+const INTERNAL_ERROR_CODE = -32603;
+const INTERNAL_ERROR_MESSAGE =
+    "AutistMask could not complete this request because of an internal error.";
+
 async function getState() {
     const result = await storageGet("autistmask");
     return (
@@ -479,13 +489,53 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
     });
 }
 
-// Detect when an approval popup (browser-action) closes without a response.
+// Anything only the extension's own pages may say. A content script speaks
-// TX and sign approvals now use windows.create() and are handled by the
+// with the page's URL, so this is what separates the popup from the site the
-// windows.onRemoved listener below, but we still handle site-connection
+// popup is being asked about.
-// approval disconnects here.
+function isExtensionSender(sender) {
+    const extUrl = runtime.getURL("");
+    return !!(sender && sender.url && sender.url.startsWith(extUrl));
+}
+
+// The approval popup's port: it carries the user's decision on a
+// site-connection approval, and its disconnect is how that approval learns the
+// popup closed without one.
+//
+// The decision travels this port rather than a one-off runtime.sendMessage()
+// for exactly one reason: the port is also what the popup's window.close()
+// disconnects. A message posted on a port is delivered before that port's
+// disconnect, so approve-then-close settles as an approval no matter how fast
+// the teardown is. Sent as a one-off message the two crossed on independent
+// channels with nothing ordering them, and the teardown won every time when
+// the prompt was driven in a tab: the user approved and the dApp was told they
+// had refused.
+//
+// TX and sign approvals do not decide here. They stay pending across a
+// disconnect — the user can reopen the toolbar popup — and are rejected by the
+// windows.onRemoved listener below.
 runtime.onConnect.addListener((port) => {
     if (port.name.startsWith("approval:")) {
         const id = port.name.split(":")[1];
+        if (pendingApprovals[id] && isExtensionSender(port.sender)) {
+            // The extension's own popup is on the other end, so its disconnect
+            // is a trustworthy "closed" and onRemoved below stands down. The
+            // sender check is what keeps that from being an off switch: a
+            // content script that guessed the id and held its port open would
+            // otherwise disable the only settlement path a prompt whose popup
+            // never connected has left, and the dApp would wait forever.
+            pendingApprovals[id].portConnected = true;
+        }
+        port.onMessage.addListener((msg) => {
+            if (!msg || msg.type !== "AUTISTMASK_APPROVAL_DECISION") return;
+            if (!isExtensionSender(port.sender)) return;
+            const approval = pendingApprovals[id];
+            if (!approval || approval.type === "tx" || approval.type === "sign")
+                return;
+            settleApproval(id, {
+                approved: !!msg.approved,
+                remember: !!msg.remember,
+            });
+        });
         port.onDisconnect.addListener(() => {
             const approval = pendingApprovals[id];
             if (approval) {
@@ -495,7 +545,6 @@ runtime.onConnect.addListener((port) => {
                 }
                 settleApproval(id, { approved: false, remember: false });
             }
-            resetPopupUrl();
         });
     }
 });
@@ -1050,10 +1099,21 @@ startBackgroundJobs();
 // outcome to the page — and the window is recorded as gone, so that an attempt
 // which then fails retryably settles instead of waiting in a window that no
 // longer exists.
+//
+// A site-connection approval whose popup connected its port is not decided
+// here. That popup approves and closes in the same breath, and this event
+// races the decision on a channel of its own — the same race the port exists
+// to end. Its port disconnect says the same thing this event does, in an order
+// that is defined, so the disconnect is left to say it. The window closing
+// before any port connected is the one case with nothing else to speak for it,
+// and is rejected here so the dApp is not left waiting on a window that is
+// gone.
 if (windowsNs && windowsNs.onRemoved) {
     windowsNs.onRemoved.addListener((windowId) => {
         for (const [id, approval] of Object.entries(pendingApprovals)) {
             if (approval.windowId !== windowId) continue;
+            const isSite = approval.type !== "tx" && approval.type !== "sign";
+            if (isSite && approval.portConnected) continue;
             const rejection = abandonedResult(
                 approval,
                 APPROVAL_REJECTED_CODE,
@@ -1079,25 +1139,40 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // keep fallback
             }
         }
-        handleRpc(msg.method, msg.params, trustedOrigin).then((response) => {
+        handleRpc(msg.method, msg.params, trustedOrigin)
-            sendResponse(response);
+            .then((response) => {
-        });
+                sendResponse(response);
+            })
+            .catch((err) => {
+                // Without this the page's window.ethereum.request() promise
+                // stays pending forever: no response is sent, the content
+                // script posts nothing back, and the dApp cannot tell the
+                // failure from a slow wallet. handleRpc does real work —
+                // state loads, provider calls, transaction population — so
+                // "it does not throw today" is not a property anyone is
+                // maintaining.
+                log.errorf("RPC request failed:", msg.method, err);
+                sendResponse({
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                });
+            });
         return true;
     }
 
     // Validate that popup-only messages originate from the extension itself.
+    // The site-connection decision is not here: it is a port message, and it
+    // is checked the same way where the port is served.
     const POPUP_ONLY_TYPES = [
         "AUTISTMASK_GET_APPROVAL",
-        "AUTISTMASK_APPROVAL_RESPONSE",
         "AUTISTMASK_TX_RESPONSE",
         "AUTISTMASK_SIGN_RESPONSE",
     ];
-    if (POPUP_ONLY_TYPES.includes(msg.type)) {
+    if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
-        const extUrl = runtime.getURL("");
+        sendResponse({ error: "Unauthorized sender" });
-        if (!sender.url || !sender.url.startsWith(extUrl)) {
+        return false;
-            sendResponse({ error: "Unauthorized sender" });
-            return false;
-        }
     }
 
     if (msg.type === "AUTISTMASK_GET_APPROVAL") {
@@ -1129,15 +1204,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
         return false;
     }
 
-    if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") {
-        settleApproval(msg.id, {
-            approved: msg.approved,
-            remember: msg.remember,
-        });
-        resetPopupUrl();
-        return false;
-    }
-
     if (msg.type === "AUTISTMASK_TX_RESPONSE") {
         const approval = pendingApprovals[msg.id];
         if (!approval) return false;
@@ -1188,6 +1254,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
             return false;
         }
 
+        // Which phase the last-resort .catch() below reports. Everything up to
+        // the broadcastTransaction() call provably never reached the network,
+        // so an escape from there must not tell the user it might have.
+        let lastResortStage = TX_STAGE_VERIFY;
         (async () => {
             // The chain this attempt is on, read once. Verification below
             // refuses an artifact signed for any other chain, and the nonce
@@ -1271,6 +1341,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
 
             try {
                 const provider = getProvider(state.rpcUrl);
+                lastResortStage = TX_STAGE_BROADCAST;
                 const tx = await provider.broadcastTransaction(msg.rawSignedTx);
                 if (nonce !== null) spent.add(nonce);
                 settleApproval(
@@ -1302,7 +1373,28 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     stage: outcome.stage,
                 });
             }
-        })();
+        })().catch((e) => {
+            // Every statement above is inside a try, but a throw from one of
+            // the catch blocks escapes as an unhandled rejection and neither
+            // the popup nor the page is ever answered. Settle both, through
+            // the same chokepoint as every other retirement.
+            log.errorf("transaction approval response failed:", e);
+            settleApproval(
+                msg.id,
+                {
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                },
+                { holdsClaim: true },
+            );
+            sendResponse({
+                error: INTERNAL_ERROR_MESSAGE,
+                retryable: false,
+                stage: lastResortStage,
+            });
+        });
         return true;
     }
 
@@ -1386,7 +1478,25 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 }
                 sendResponse({ error: errMsg, retryable });
             }
-        })();
+        })().catch((e) => {
+            // Same shape as the transaction path: a throw out of the catch
+            // block above would leave the popup and the page both waiting.
+            log.errorf("sign approval response failed:", e);
+            settleApproval(
+                msg.id,
+                {
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                },
+                { holdsClaim: true },
+            );
+            sendResponse({
+                error: INTERNAL_ERROR_MESSAGE,
+                retryable: false,
+            });
+        });
         return true;
     }
 

src/popup/views/approval.js

@@ -443,7 +443,7 @@ function showSignApproval(details) {
 // describe the approval is the same outcome as an approval that is gone.
 async function show(id) {
     approvalId = id;
-    runtimeApi().connect({ name: "approval:" + id });
+    approvalPort = runtimeApi().connect({ name: "approval:" + id });
 
     let details = null;
     try {
@@ -476,6 +476,14 @@ async function show(id) {
 }
 
 let approvalId = null;
+// The port this approval was opened on. Closing this window disconnects it,
+// and the background treats that disconnect as "closed without deciding" for a
+// site connection — so the decision goes out on this same port and not as a
+// one-off message. One channel is ordered: a message posted on it is delivered
+// before its own disconnect, however immediately the close follows. Two
+// channels were not, and the close won, reporting a user who approved as
+// having refused.
+let approvalPort = null;
 let pendingTxDetails = null;
 // The exact objects shown to the user, kept so the popup signs what it
 // displayed rather than re-fetching or re-populating anything at approval
@@ -543,6 +551,28 @@ function clearSignPassword() {
     hideError("approve-sign-error");
 }
 
+// Answer a site-connection approval and close. The decision goes out on the
+// approval port — see approvalPort above for why — and carries no approval id,
+// because the port name already names the approval the background will settle.
+// The post is guarded because a throw must not cost the close: posting on a
+// port whose background worker has been torn down throws, and the approval it
+// would have settled died with that worker, so the only thing left to do is
+// what the user asked for — go away.
+function decideSite(approved) {
+    if (approvalPort) {
+        try {
+            approvalPort.postMessage({
+                type: "AUTISTMASK_APPROVAL_DECISION",
+                approved,
+                remember: $("approve-remember").checked,
+            });
+        } catch {
+            // Nothing to report it to; the window closes either way.
+        }
+    }
+    window.close();
+}
+
 function init(_ctx) {
     onViewLeave("approve-tx", clearTxPassword);
     onViewLeave("approve-sign", clearSignPassword);
@@ -553,25 +583,11 @@ function init(_ctx) {
     });
 
     $("btn-approve").addEventListener("click", () => {
-        const remember = $("approve-remember").checked;
+        decideSite(true);
-        notify({
-            type: "AUTISTMASK_APPROVAL_RESPONSE",
-            id: approvalId,
-            approved: true,
-            remember,
-        });
-        window.close();
     });
 
     $("btn-reject").addEventListener("click", () => {
-        const remember = $("approve-remember").checked;
+        decideSite(false);
-        notify({
-            type: "AUTISTMASK_APPROVAL_RESPONSE",
-            id: approvalId,
-            approved: false,
-            remember,
-        });
-        window.close();
     });
 
     $("btn-approve-tx").addEventListener("click", async () => {

src/popup/views/settings.js

@@ -167,10 +167,7 @@ function renderWalletListSettings() {
 function show() {
     $("settings-rpc").value = state.rpcUrl;
     $("settings-blockscout").value = state.blockscoutUrl;
-    const networkSelect = $("settings-network");
+    $("settings-network").value = state.networkId;
-    if (networkSelect) {
-        networkSelect.value = state.networkId;
-    }
     renderTrackedTokens();
     renderSiteLists();
     renderWalletListSettings();
@@ -282,15 +279,13 @@ function init(ctx) {
     });
 
     const networkSelect = $("settings-network");
-    if (networkSelect) {
+    networkSelect.addEventListener("change", async () => {
-        networkSelect.addEventListener("change", async () => {
+        const newId = networkSelect.value;
-            const newId = networkSelect.value;
+        const net = await onChainSwitch(newId);
-            const net = await onChainSwitch(newId);
+        $("settings-rpc").value = state.rpcUrl;
-            $("settings-rpc").value = state.rpcUrl;
+        $("settings-blockscout").value = state.blockscoutUrl;
-            $("settings-blockscout").value = state.blockscoutUrl;
+        showFlash("Switched to " + net.name + ".");
-            showFlash("Switched to " + net.name + ".");
+    });
-        });
-    }
 
     $("settings-show-zero-balances").checked = state.showZeroBalanceTokens;
     $("settings-show-zero-balances").addEventListener("change", async () => {

tests/backgroundApproval.test.js

@@ -20,6 +20,11 @@
 
 const { Network, Wallet } = require("ethers");
 
+// The real formatter the approval screen renders failures through. Bound here,
+// before any jest.doMock() of the module, so the copy assertions below check
+// what the user is actually shown.
+const { describeSigningFailure } = require("../src/shared/approvalVerify");
+
 const SIGNER_KEY =
     "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
 const OTHER_KEY =
@@ -34,6 +39,21 @@ const HOSTNAME = "dapp.example";
 const UNCONNECTED_ORIGIN = "https://stranger.example";
 const EXT_URL = "chrome-extension://autistmask/";
 
+// An origin the persisted state has never allowed, so asking to connect from
+// it raises a prompt rather than being answered from allowedSites.
+const FRESH_ORIGIN = "https://fresh.example";
+
+// The approval id in the most recent popup URL of a list, or null when none
+// of them carries one. Takes both shapes: the absolute URL windows.create()
+// is given and the extension-relative one action.setPopup() is given.
+function approvalIdIn(urls) {
+    for (let i = urls.length - 1; i >= 0; i--) {
+        if (!urls[i] || !urls[i].includes("?approval=")) continue;
+        return new URL(urls[i], EXT_URL).searchParams.get("approval");
+    }
+    return null;
+}
+
 // What the dApp asks for: no nonce, no gas, no fees. This is the shape that
 // makes a duplicate broadcast possible at all.
 const TX_PARAMS = {
@@ -144,6 +164,14 @@ function loadBackground(options) {
         ensureRecurringAlarms: jest.fn(async () => {}),
         registerAlarmHandlers: jest.fn(),
     }));
+    // The real verification module, except where a test replaces one export
+    // with a throw to drive the handler's own error handling into failing.
+    if (opts.approvalVerify) {
+        jest.doMock("../src/shared/approvalVerify", () => ({
+            ...jest.requireActual("../src/shared/approvalVerify"),
+            ...opts.approvalVerify,
+        }));
+    }
 
     const persisted = {
         wallets: [
@@ -157,13 +185,21 @@ function loadBackground(options) {
 
     let messageListener = null;
     let windowRemovedListener = null;
+    let connectListener = null;
     const created = [];
     const removed = [];
+    // Every URL the background put on the browser action. A site approval
+    // raised through action.openPopup() opens no window at all, so this is
+    // the only place its id appears.
+    const actionPopups = [];
 
     global.chrome = {
         storage: {
             local: {
-                get: jest.fn(async () => ({ autistmask: persisted })),
+                get: jest.fn(
+                    opts.storageGet ||
+                        (async () => ({ autistmask: persisted })),
+                ),
                 set: jest.fn(async () => {}),
             },
         },
@@ -174,7 +210,14 @@ function loadBackground(options) {
                     messageListener = fn;
                 },
             },
-            onConnect: { addListener: () => {} },
+            // Captured, not swallowed: the approval port is what carries a
+            // site connection's decision and the popup teardown that races
+            // it, so a no-op stub here hides the whole subject of #275.
+            onConnect: {
+                addListener: (fn) => {
+                    connectListener = fn;
+                },
+            },
             lastError: null,
         },
         windows: {
@@ -202,7 +245,17 @@ function loadBackground(options) {
             query: (q, cb) => cb([]),
             sendMessage: () => {},
         },
-        action: { setPopup: () => {} },
+        action: {
+            setPopup: (o) => {
+                actionPopups.push(o.popup);
+            },
+            // The production route for a site connection. Present only when
+            // a test asks for it, because with it the prompt is the toolbar
+            // popup: no window is created, so windows.onRemoved can never
+            // fire for it and the port disconnect is the only close signal
+            // that exists.
+            ...(opts.actionPopup ? { openPopup: () => Promise.resolve() } : {}),
+        },
     };
 
     require("../src/background/index");
@@ -270,6 +323,70 @@ function loadBackground(options) {
         };
     }
 
+    // A dApp asking to connect. The origin defaults to one the persisted
+    // state has never allowed, so the request really does raise a prompt
+    // instead of being answered from allowedSites.
+    function requestSite(origin) {
+        let rpcResult = null;
+        messageListener(
+            {
+                type: "AUTISTMASK_RPC",
+                method: "eth_requestAccounts",
+                params: [],
+            },
+            { origin: origin || FRESH_ORIGIN },
+            (r) => {
+                rpcResult = r;
+            },
+        );
+        return {
+            // Wherever the prompt went: the toolbar popup URL when
+            // action.openPopup() carried it, the created window otherwise.
+            id: () =>
+                approvalIdIn(actionPopups) ||
+                approvalIdIn(created.map((c) => c.url)),
+            result: () => rpcResult,
+        };
+    }
+
+    // The popup's approval port, as the browser delivers it. Messages posted
+    // on a port and that port's disconnect travel one channel in FIFO order,
+    // which is exactly the property the fix rests on, so this stub delivers
+    // them in the order the caller emits them and never reorders them.
+    function connectApproval(id, senderUrl) {
+        const onMessage = [];
+        const onDisconnect = [];
+        const port = {
+            name: "approval:" + id,
+            sender: {
+                url:
+                    senderUrl === undefined
+                        ? EXT_URL + "src/popup/index.html?approval=" + id
+                        : senderUrl,
+            },
+            onMessage: { addListener: (fn) => onMessage.push(fn) },
+            onDisconnect: { addListener: (fn) => onDisconnect.push(fn) },
+        };
+        connectListener(port);
+        return {
+            decide: (approved, remember) => {
+                for (const fn of onMessage) {
+                    fn(
+                        {
+                            type: "AUTISTMASK_APPROVAL_DECISION",
+                            approved,
+                            remember: !!remember,
+                        },
+                        port,
+                    );
+                }
+            },
+            disconnect: () => {
+                for (const fn of onDisconnect) fn(port);
+            },
+        };
+    }
+
     // The user closes the approval popup. `created` is index-aligned with the
     // ids the window stub hands back, so window 1 is the first popup opened.
     function closeWindow(windowId) {
@@ -280,6 +397,8 @@ function loadBackground(options) {
         send,
         requestTx,
         requestSign,
+        requestSite,
+        connectApproval,
         closeWindow,
         broadcastTransaction,
         loadState,
@@ -306,6 +425,15 @@ async function settle() {
     for (let i = 0; i < 50; i++) await Promise.resolve();
 }
 
+// settle() only drains microtasks. A handler whose last-resort .catch() has to
+// run after a macrotask boundary needs those turns too, so the assertion that
+// the page WAS answered is what reports a regression rather than a timeout.
+async function settleIncludingRejections() {
+    await settle();
+    await new Promise((resolve) => setImmediate(resolve));
+    await new Promise((resolve) => setImmediate(resolve));
+}
+
 afterEach(() => {
     delete global.chrome;
     jest.resetModules();
@@ -1372,6 +1500,200 @@ describe("a claimed approval outlives every other retirement path", () => {
     });
 });
 
+// A handler that throws must still answer. `sendResponse` is the only thing
+// that settles the page's window.ethereum.request() promise, so a throw that
+// escapes a handler leaves that promise pending forever — no error, no
+// timeout, indistinguishable from a slow wallet. Each case below drives a real
+// throw out of a handler rather than asserting the catch block exists.
+describe("a handler that throws still settles the page", () => {
+    const INTERNAL_ERROR = {
+        code: -32603,
+        message:
+            "AutistMask could not complete this request because of an internal error.",
+    };
+
+    let errorLog;
+    beforeEach(() => {
+        errorLog = jest.spyOn(console, "error").mockImplementation(() => {});
+    });
+    afterEach(() => {
+        errorLog.mockRestore();
+    });
+
+    // getState() awaits extension storage unguarded, and every read path in
+    // handleRpc goes through it. A storage read that rejects is the whole
+    // failure — no hook in the handler itself.
+    test("a rejected handleRpc rejects the page instead of hanging it", async () => {
+        const bg = loadBackground({
+            storageGet: async () => {
+                throw new Error("storage unavailable");
+            },
+        });
+
+        const answer = bg.send(
+            { type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] },
+            { origin: ORIGIN },
+        );
+        await settleIncludingRejections();
+
+        // The channel is held open for the async answer, and the answer
+        // arrives.
+        expect(answer.kept).toBe(true);
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR,
+        });
+        // Not swallowed: the throw is on the background console, which is how
+        // this class gets caught in future.
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "RPC request failed:",
+            "eth_accounts",
+            expect.objectContaining({ message: "storage unavailable" }),
+        );
+    });
+
+    // The transaction response handler wraps every statement in a try, so what
+    // escapes it is a throw from inside one of its catch blocks. Here the
+    // failure classifier itself throws while classifying a real verification
+    // failure — the approval is left claimed, so nothing else can settle it.
+    // The escape happens before broadcastTransaction() is reached, so the
+    // reported stage must be the one that says the transaction is gone.
+    test("a throw while verifying a transaction settles both the page and the popup", async () => {
+        const bg = loadBackground({
+            approvalVerify: {
+                describeTxFailure: () => {
+                    throw new Error("classifier broke");
+                },
+            },
+        });
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        // A real verification failure: the artifact is signed at a nonce the
+        // approval never displayed.
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(NONCE + 1),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settleIncludingRejections();
+
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR.message,
+            retryable: false,
+            // Nothing was broadcast, so the popup must say the request is gone
+            // rather than that it may still have reached the network.
+            stage: "verify",
+        });
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "transaction approval response failed:",
+            expect.objectContaining({ message: "classifier broke" }),
+        );
+        // The copy the user actually reads, from the popup's own formatter.
+        expect(
+            describeSigningFailure(answer.sendResponse.mock.calls[0][0], "")
+                .message,
+        ).toBe(
+            INTERNAL_ERROR.message +
+                " This request can no longer be signed." +
+                " Please start it again from the site.",
+        );
+    });
+
+    // The other side of the same local: once broadcastTransaction() has been
+    // entered the wallet genuinely cannot tell whether the node took the
+    // transaction, and the copy that warns about a second send is correct.
+    test("a throw while handling a failed broadcast reports the broadcast stage", async () => {
+        const bg = loadBackground({
+            approvalVerify: {
+                describeTxFailure: () => {
+                    throw new Error("classifier broke");
+                },
+            },
+        });
+        bg.broadcastTransaction.mockRejectedValue(new Error("node refused"));
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        // The approved artifact, so verification passes and the failure
+        // happens at the broadcast.
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(NONCE),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settleIncludingRejections();
+
+        expect(bg.broadcastTransaction).toHaveBeenCalled();
+        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR.message,
+            retryable: false,
+            stage: "broadcast",
+        });
+        expect(
+            describeSigningFailure(answer.sendResponse.mock.calls[0][0], "")
+                .message,
+        ).toBe(
+            INTERNAL_ERROR.message +
+                " The transaction may still have reached the network." +
+                " Check the account before sending it again.",
+        );
+    });
+
+    test("a throw while handling a failed signature settles both the page and the popup", async () => {
+        const bg = loadBackground({
+            approvalVerify: {
+                failureIsRetryable: () => {
+                    throw new Error("classifier broke");
+                },
+            },
+        });
+        const pending = bg.requestSign();
+        await settle();
+
+        // A real verification failure: the active address moved after the
+        // approval was raised.
+        bg.setActiveAddress(other.address);
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id: pending.id(),
+                approved: true,
+                signature: await signer.signMessage(
+                    Buffer.from(MESSAGE.slice(2), "hex"),
+                ),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settleIncludingRejections();
+
+        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR.message,
+            retryable: false,
+        });
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "sign approval response failed:",
+            expect.objectContaining({ message: "classifier broke" }),
+        );
+    });
+});
+
 describe("popup-only messages", () => {
     test("a page sender cannot answer an approval", async () => {
         const bg = loadBackground();
@@ -1396,3 +1718,197 @@ describe("popup-only messages", () => {
         });
     });
 });
+
+// A site connection decided in a popup that closes on the next line.
+//
+// The decision and the teardown are two events the popup emits back to back,
+// and the background must not be able to reach different outcomes depending on
+// which of them it processes first. It cannot, because they are now one
+// channel: the decision is posted on the approval port that the close then
+// disconnects, so it is delivered first. Every test here therefore emits the
+// close IMMEDIATELY after the decision, with nothing awaited in between —
+// which is what the popup does, and what used to report a user who approved as
+// having refused (#275).
+describe("a site connection decided as the popup closes", () => {
+    // The production route: chrome.action.openPopup() put the prompt in the
+    // toolbar popup, which is not a window, so nothing but the port
+    // disconnect can tell the background this prompt is gone.
+    test("approving in the toolbar popup connects the site", async () => {
+        const bg = loadBackground({ actionPopup: true });
+        const pending = bg.requestSite();
+        await settle();
+        const id = pending.id();
+        expect(id).toBeTruthy();
+        expect(bg.created).toHaveLength(0);
+
+        const port = bg.connectApproval(id);
+        port.decide(true, false);
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({ result: [signer.address] });
+    });
+
+    test("closing the toolbar popup without deciding is a rejection", async () => {
+        const bg = loadBackground({ actionPopup: true });
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id());
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    test("rejecting is a rejection, and the close that follows adds nothing", async () => {
+        const bg = loadBackground({ actionPopup: true });
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id());
+        port.decide(false, false);
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    // The port carries a decision now, so it carries the sender check the
+    // one-off message used to carry. A content script that guessed an
+    // approval id must not be able to connect the site it is running on.
+    test("a decision from a page sender is ignored, and the close rejects", async () => {
+        const bg = loadBackground({ actionPopup: true });
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id(), FRESH_ORIGIN + "/x.html");
+        port.decide(true, true);
+        await settle();
+        expect(pending.result()).toBeNull();
+
+        port.disconnect();
+        await settle();
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    // The fallback shape, where openPopup() is unavailable and the prompt is
+    // a window the extension opened. Closing it fires windows.onRemoved as
+    // well, on a channel of its own that is ordered against nothing — so the
+    // window event must not be allowed to decide a site approval either.
+    //
+    // The event goes FIRST here, which is the interleaving the guard in the
+    // onRemoved listener exists for: the approval is still pending when the
+    // event arrives, so the listener really reaches it and really has to
+    // decline it. With the decision first there is nothing left in
+    // pendingApprovals and the listener finds no approval to spare.
+    test("approving in the fallback window survives a window event that lands first", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSite();
+        await settle();
+        expect(bg.created).toHaveLength(1);
+
+        const port = bg.connectApproval(pending.id());
+        bg.closeWindow(1);
+        port.decide(true, false);
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({ result: [signer.address] });
+    });
+
+    test("approving in the fallback window survives a window event that follows", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id());
+        port.decide(true, false);
+        bg.closeWindow(1);
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({ result: [signer.address] });
+    });
+
+    // The connected port is what silences the window event, so connecting one
+    // must take the same sender check the decision takes. Otherwise a content
+    // script that guessed the id switches off the only settlement path a
+    // prompt whose real popup never connected has, and the dApp hangs.
+    test("a port from a page sender does not silence the window event", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSite();
+        await settle();
+
+        // Connected and held open — no disconnect, so nothing but the window
+        // event can settle this approval.
+        bg.connectApproval(pending.id(), FRESH_ORIGIN + "/x.html");
+        bg.closeWindow(1);
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    // Same shape, and the same window event arriving before the popup has
+    // said anything at all — which is a user closing the window rather than
+    // deciding, and still has to reach the dApp as a rejection.
+    test("closing the fallback window without deciding is a rejection", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id());
+        bg.closeWindow(1);
+        port.disconnect();
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    // The net under the paragraph above: a prompt whose page never got as far
+    // as connecting the port has no disconnect to reject it, so the window
+    // event has to. Otherwise the dApp waits forever on a window that is gone.
+    test("a window that closes before its popup ever connected still rejects", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSite();
+        await settle();
+
+        bg.closeWindow(1);
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+
+    // The `isSite &&` half of that skip, which is what keeps it from reaching
+    // a tx or sign approval. The popup connects its port in show() before it
+    // knows the approval's type, and the background sets portConnected without
+    // looking at the type either, so a tx approval in the fallback window
+    // carries the flag too. Without the conjunct the window event would skip
+    // it, windowClosed would never be set, releaseApproval() would never settle
+    // it, and the page would hang — the #271 regression this guard is written
+    // around.
+    test("a tx window closed with the port connected still rejects", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        bg.connectApproval(pending.id());
+        await settle();
+        bg.closeWindow(1);
+        await settle();
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+    });
+});

tests/e2e/run.js

@@ -43,6 +43,7 @@ const {
     STUB_TX_HASH,
 } = require("./network");
 const { DUST_THRESHOLD_MESSAGE } = require("../../src/popup/dustThreshold");
+const { NETWORKS } = require("../../src/shared/networks");
 
 const TEST_TIMEOUT_MS = 120000;
 
@@ -945,6 +946,328 @@ test("confirming removes the address and returns Home (#162)", async (env) => {
     );
 });
 
+// ------------------------------------------------- Settings screen (#229)
+
+// Settings holds the densest run of $("...") lookups in the codebase, and
+// until this section nothing drove it in a browser. One wrong id makes
+// settings.init() throw, which aborts the rest of index.js init() before it
+// renders anything at all — so a broken id does not degrade Settings, it
+// leaves the whole popup blank. These tests assert the controls are there
+// AND that they work, because "the view is visible" would still pass
+// against a screen whose handlers were never wired.
+
+// The four Token Spam Protection checkboxes, in markup order, with the
+// src/shared/state.js key each one is bound to. All four default true.
+const SPAM_FILTER_CHECKBOXES = [
+    { id: "settings-hide-spoofed-symbols", key: "hideSpoofedSymbols" },
+    { id: "settings-hide-low-holders", key: "hideLowHolderTokens" },
+    { id: "settings-hide-fraud-contracts", key: "hideFraudContracts" },
+    { id: "settings-hide-dust", key: "hideDustTransactions" },
+];
+
+// The one toggled through a reopen. Chosen because nothing later in this
+// suite depends on it: the other three filter token and transaction lists
+// that the ConfirmTx and dApp sections go on to drive.
+const TOGGLED_FILTER = "settings-hide-dust";
+
+// Everything the Settings assertions below must observe, recorded as each
+// group of them completes. The final test demands the exact set.
+//
+// The point is that a green run cannot mean the assertions were skipped.
+// Navigation that silently fails already fails a test — visible() throws
+// on a timeout — but an early return, a deleted test, or a body that
+// stopped being reached would otherwise shrink this section quietly
+// instead of reddening the run.
+const SETTINGS_COVERAGE = [
+    "about-well",
+    "spam-checkbox-defaults",
+    "theme-select",
+    "network-select",
+    "selector-round-trip",
+    "selector-restore",
+    "toggle-off-survives-reopen",
+    "toggle-on-survives-reopen",
+    "wallet-list",
+];
+
+// A control read as the DOM has it, not as a selector claims: tag name and
+// type distinguish a real <input type="checkbox"> from a <div> that merely
+// carries the id, and `checked` is the live property rather than the
+// attribute, so it reflects what init() assigned.
+function controlState(page, id) {
+    return page.evaluate((elementId) => {
+        const el = document.getElementById(elementId);
+        if (!el) return null;
+        return {
+            tag: el.tagName.toLowerCase(),
+            type: el.type || "",
+            checked: el.checked,
+            value: el.value,
+            options: Array.from(el.options || []).map((o) => o.value),
+        };
+    }, id);
+}
+
+async function checkboxStates(page) {
+    const out = {};
+    for (const { id } of SPAM_FILTER_CHECKBOXES) {
+        out[id] = await controlState(page, id);
+    }
+    return out;
+}
+
+function assertSpamCheckbox(st, id, expected, where) {
+    assert(st !== null, "no element with id " + id + " on Settings " + where);
+    assert(
+        st.tag === "input" && st.type === "checkbox",
+        id + " is a <" + st.tag + " type=" + st.type + ">, not a checkbox",
+    );
+    assert(
+        st.checked === expected,
+        id +
+            " reads " +
+            st.checked +
+            " " +
+            where +
+            ", expected " +
+            expected +
+            " — the checkbox is on screen but not carrying the persisted value",
+    );
+}
+
+test("Settings renders with the whole screen populated (#229)", async (env) => {
+    await visible(env.page, "#view-main");
+    await openSettings(env.page);
+
+    // show() writes the About well near its end — only the debug well and
+    // the debug-mode checkbox follow it — and showView() is the last thing
+    // of all, so an id show() cannot find aborts before Settings is ever
+    // displayed. Reading these values back proves show() ran through to
+    // there, not just far enough to unhide the section. They are filled
+    // from build-time constants that always have a value, so empty means
+    // the write did not happen.
+    const about = await env.page.evaluate(() => {
+        const out = {};
+        for (const id of [
+            "about-license",
+            "about-author",
+            "about-version",
+            "about-release-date",
+            "about-commit-link",
+        ]) {
+            const el = document.getElementById(id);
+            out[id] = el === null ? null : el.textContent.trim();
+        }
+        return out;
+    });
+    for (const [id, text] of Object.entries(about)) {
+        assert(
+            text !== null && text.length > 0,
+            "the About well left #" +
+                id +
+                " unwritten: " +
+                JSON.stringify(about),
+        );
+    }
+    env.settingsCoverage.add("about-well");
+
+    // The wallet list is rendered by settings.js rather than authored in
+    // index.html, so an empty container means renderWalletListSettings()
+    // did not run even though the screen came up.
+    const wallets = await env.page
+        .locator("#settings-wallet-list .settings-wallet-name")
+        .count();
+    assert(
+        wallets >= 2,
+        "Settings lists " +
+            wallets +
+            " wallets; the suite created two by this point",
+    );
+    env.settingsCoverage.add("wallet-list");
+});
+
+test("the four Token Spam Protection checkboxes render, defaulted on (#229)", async (env) => {
+    await openSettings(env.page);
+    const states = await checkboxStates(env.page);
+    for (const { id } of SPAM_FILTER_CHECKBOXES) {
+        assertSpamCheckbox(states[id], id, true, "on first render");
+    }
+    env.settingsCoverage.add("spam-checkbox-defaults");
+});
+
+test("the theme and network selectors render their real choices (#229)", async (env) => {
+    await openSettings(env.page);
+
+    const theme = await controlState(env.page, "settings-theme");
+    assert(theme !== null, "no #settings-theme element on Settings");
+    assert(
+        theme.tag === "select",
+        "#settings-theme is a <" + theme.tag + ">, not a <select>",
+    );
+    assert(
+        theme.options.join(",") === "system,light,dark",
+        "the theme selector offers " + JSON.stringify(theme.options),
+    );
+    env.settingsCoverage.add("theme-select");
+
+    const network = await controlState(env.page, "settings-network");
+    assert(network !== null, "no #settings-network element on Settings");
+    assert(
+        network.tag === "select",
+        "#settings-network is a <" + network.tag + ">, not a <select>",
+    );
+    const wantNetworks = Object.keys(NETWORKS).sort().join(",");
+    assert(
+        network.options.slice().sort().join(",") === wantNetworks,
+        "the network selector offers " +
+            JSON.stringify(network.options) +
+            ", expected the networks in src/shared/networks.js: " +
+            wantNetworks,
+    );
+    env.settingsCoverage.add("network-select");
+});
+
+// Reads both selectors in one page task, so a round trip cannot observe
+// them at two different moments.
+async function selectorValues(page) {
+    const theme = await controlState(page, "settings-theme");
+    const network = await controlState(page, "settings-network");
+    assert(theme !== null, "no #settings-theme element on Settings");
+    assert(network !== null, "no #settings-network element on Settings");
+    return { theme: theme.value, network: network.value };
+}
+
+function assertSelectors(got, wantTheme, wantNetwork, where) {
+    assert(
+        got.theme === wantTheme,
+        "the theme selector shows " +
+            JSON.stringify(got.theme) +
+            " " +
+            where +
+            ", expected " +
+            JSON.stringify(wantTheme),
+    );
+    assert(
+        got.network === wantNetwork,
+        "the network selector shows " +
+            JSON.stringify(got.network) +
+            " " +
+            where +
+            ", expected " +
+            JSON.stringify(wantNetwork),
+    );
+}
+
+// The two values the selectors are driven to. NEITHER is the first
+// <option> of its <select> (`system` and `mainnet` are), and that is the
+// entire point: the first option is what the DOM reports with no
+// JavaScript involved at all, so asserting it would pass just as happily
+// against a Settings screen that never assigned anything. Only a value
+// that went out through the change handler and saveState(), and came back
+// through loadState() and the assignment show()/init() makes, can be read
+// here.
+const NONDEFAULT_THEME = "dark";
+const NONDEFAULT_NETWORK = "sepolia";
+
+test("the theme and network selectors carry a non-default persisted value (#229)", async (env) => {
+    await openSettings(env.page);
+
+    // selectOption() fires "change", which is what the handlers bind.
+    await env.page.selectOption("#settings-theme", NONDEFAULT_THEME);
+    await env.page.selectOption("#settings-network", NONDEFAULT_NETWORK);
+
+    await reopenPopup(env, "#view-settings");
+
+    assertSelectors(
+        await selectorValues(env.page),
+        NONDEFAULT_THEME,
+        NONDEFAULT_NETWORK,
+        "after reopening the popup",
+    );
+    env.settingsCoverage.add("selector-round-trip");
+
+    // Restore, the same way round, and assert the restore actually took
+    // rather than trusting it: the later sections inherit this fixture,
+    // and a selector stuck on `dark`/`sepolia` would otherwise be
+    // indistinguishable here from one that persists correctly. Switching
+    // the network back also returns state.rpcUrl and state.blockscoutUrl
+    // to the mainnet defaults that onChainSwitch() overwrote, which are
+    // the values src/shared/state.js starts with.
+    await env.page.selectOption("#settings-theme", "system");
+    await env.page.selectOption("#settings-network", "mainnet");
+
+    await reopenPopup(env, "#view-settings");
+
+    assertSelectors(
+        await selectorValues(env.page),
+        "system",
+        "mainnet",
+        "after restoring and reopening the popup",
+    );
+    env.settingsCoverage.add("selector-restore");
+});
+
+// The functional half. A checkbox that renders but is not wired looks
+// identical on screen; only a value that survives being written to storage
+// and read back by a fresh page load tells the two apart. That round trip
+// runs through the change handler, saveState(), loadState() and the
+// assignment init() makes — every part of the wiring at once.
+test("a spam filter toggled in Settings survives a popup reopen (#229)", async (env) => {
+    await openSettings(env.page);
+    await env.page.click("#" + TOGGLED_FILTER);
+
+    const immediately = await controlState(env.page, TOGGLED_FILTER);
+    assert(
+        immediately.checked === false,
+        "clicking #" + TOGGLED_FILTER + " did not clear it",
+    );
+
+    await reopenPopup(env, "#view-settings");
+
+    const after = await checkboxStates(env.page);
+    for (const { id } of SPAM_FILTER_CHECKBOXES) {
+        assertSpamCheckbox(
+            after[id],
+            id,
+            id !== TOGGLED_FILTER,
+            "after reopening the popup",
+        );
+    }
+    env.settingsCoverage.add("toggle-off-survives-reopen");
+});
+
+test("turning the same filter back on survives a reopen too (#229)", async (env) => {
+    await openSettings(env.page);
+    await env.page.click("#" + TOGGLED_FILTER);
+
+    await reopenPopup(env, "#view-settings");
+
+    // Restores the fixture the later sections inherit, and rules out a
+    // checkbox that persists "off" only because it is stuck there.
+    const after = await checkboxStates(env.page);
+    for (const { id } of SPAM_FILTER_CHECKBOXES) {
+        assertSpamCheckbox(after[id], id, true, "after toggling back on");
+    }
+    env.settingsCoverage.add("toggle-on-survives-reopen");
+
+    await env.page.click("#btn-settings-back");
+    await visible(env.page, "#view-main");
+});
+
+test("the Settings assertions above all ran (#229)", async (env) => {
+    const seen = [...env.settingsCoverage].sort();
+    const want = SETTINGS_COVERAGE.slice().sort();
+    assert(
+        seen.join(",") === want.join(","),
+        "the Settings section covered " +
+            JSON.stringify(seen) +
+            " but must cover " +
+            JSON.stringify(want) +
+            " — a green run here would otherwise mean only that fewer " +
+            "assertions ran, not that they passed",
+    );
+});
+
 // ------------------------------------------------ dust threshold (#233)
 
 // The popup size README documents the UI as designed for. Pages in this
@@ -1861,32 +2184,13 @@ async function reserveApprovalTab(env) {
     // one down with it.
     env.approvalTab = await env.ctx.newPage();
 
-    // The one accommodation this section makes to the shipped code, and the
+    // This tab runs the shipped popup with nothing patched. The site
-    // reason for it.
+    // approval buttons decide and then close on the next line, and the two
-    //
+    // site-approval tests below are therefore the real-browser
-    // Both approval buttons call runtime.sendMessage() and then window.close()
+    // approve-then-immediate-close and reject-then-immediate-close cases: the
-    // on the next line. Closing this page disconnects the approval port, and
+    // decision rides the approval port, which also carries the disconnect the
-    // the disconnect handler in src/background/index.js settles a pending
+    // close causes, so it is delivered ahead of it and the outcome does not
-    // site approval as a rejection. In a tab those two race and the teardown
+    // depend on the teardown timing (#275).
-    // wins: the approve message is never acted on, and the page is told the
-    // user rejected. Measured — with the close left in place the approval
-    // resolves as a rejection every time; with it deferred it resolves as an
-    // approval every time.
-    //
-    // It is deferred, not removed: the harness closes the page itself once
-    // the outcome has been observed, which is what window.close() would have
-    // done, only after the message it was racing has been processed.
-    //
-    // This affects the site-connection prompt only. The sign and transaction
-    // prompts run in windows the extension opens itself, with window.close()
-    // untouched, and their disconnect handler deliberately keeps a tx or sign
-    // approval pending rather than rejecting it — so there is no race there
-    // to accommodate. Whether the same ordering holds in a real toolbar popup
-    // is not observable from a headless harness and is reported rather than
-    // assumed either way.
-    await env.approvalTab.addInitScript(() => {
-        window.close = function () {};
-    });
     await env.approvalTab.goto("about:blank");
     await sleep(APPROVAL_TAB_SETTLE_MS);
     return env.approvalTab;
@@ -1935,6 +2239,95 @@ async function closeApprovalPages(ctx) {
     }
 }
 
+// Click a button whose own handler closes the window it lives in — every
+// Reject, and Allow on the site prompt.
+//
+// page.click() dispatches the click and then waits for the renderer to
+// acknowledge it, and a page torn down by the handler never gets to. The
+// dispatch is what the test needs and the log shows it happening ("performing
+// click action") immediately before the failure; the page going away is the
+// button working, not the click failing. Observed on #btn-reject-sign and
+// #btn-reject-tx, whose windows have always closed themselves.
+//
+// What the swallow costs is not the same for every button, so neither is what
+// proves the click landed:
+//
+//   #btn-reject-sign, #btn-reject-tx — their disconnect leaves the approval
+//     pending, so a click that never landed leaves the dApp promise unsettled
+//     and the assertion after the call fails on its own.
+//   #btn-approve — only a decision resolves the promise, and a swallowed click
+//     cannot produce settled === "resolved".
+//   #btn-reject on the site prompt — NOT self-proving. A page that went away
+//     without the click landing disconnects the approval port, the background
+//     settles that as 4001, and 4001 is exactly what assertUserRejection
+//     accepts. That call site arms the click trace below and asserts it.
+//
+// A button that is missing or unclickable raises a different error, which is
+// rethrown.
+async function clickAndClose(page, selector) {
+    try {
+        await page.click(selector);
+    } catch (e) {
+        if (!String((e && e.message) || e).includes("has been closed")) throw e;
+    }
+}
+
+// Evidence that a click reached the button, for the button whose outcome
+// cannot tell.
+//
+// A capture-phase listener on the document runs ahead of the button's own
+// handler and writes one key with localStorage.setItem(), which is synchronous
+// and therefore already in the browser process when the handler tears the page
+// down a line later. Any other page of the extension origin can read it back,
+// and env.page is one. The listener only observes: nothing about the shipped
+// decide-then-close is deferred, patched or reordered.
+const CLICK_TRACE_KEY = "autistmask-e2e-click-landed";
+
+async function armClickTrace(env, page, selector) {
+    await env.page.evaluate(
+        (key) => localStorage.removeItem(key),
+        CLICK_TRACE_KEY,
+    );
+    await page.evaluate(
+        ({ key, sel }) => {
+            document.addEventListener(
+                "click",
+                (e) => {
+                    const target = e.target;
+                    if (target && target.closest && target.closest(sel)) {
+                        localStorage.setItem(key, sel);
+                    }
+                },
+                true,
+            );
+        },
+        { key: CLICK_TRACE_KEY, sel: selector },
+    );
+}
+
+// The write crosses processes to reach env.page's renderer, so it is waited
+// for rather than read once. Nothing else in the test is timed on this.
+async function assertClickLanded(env, selector, timeout = 5000) {
+    const deadline = Date.now() + timeout;
+    let seen;
+    for (;;) {
+        seen = await env.page.evaluate(
+            (key) => localStorage.getItem(key),
+            CLICK_TRACE_KEY,
+        );
+        if (seen === selector || Date.now() > deadline) break;
+        await sleep(25);
+    }
+    assert(
+        seen === selector,
+        "the click on " +
+            selector +
+            " never reached the button, so the outcome below proves nothing " +
+            "about it: trace was " +
+            JSON.stringify(seen),
+    );
+}
+
 // Record every message the approval window sends to the background worker.
 //
 // This is the direct observation the password check needs. It is installed
@@ -2164,7 +2557,11 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as
         // origin in deniedSites and every later test in this section is
         // auto-rejected with no prompt at all, which would look like a pass.
         await popup.uncheck("#approve-remember");
-        await popup.click("#btn-reject");
+        // The rejection this asserts is also what an unclicked prompt that
+        // simply went away produces, so the click itself is witnessed.
+        await armClickTrace(env, popup, "#btn-reject");
+        await clickAndClose(popup, "#btn-reject");
+        await assertClickLanded(env, "#btn-reject");
 
         await assertUserRejection(
             env.dapp,
@@ -2195,7 +2592,7 @@ test("eth_requestAccounts approved returns the selected address (#183)", async (
         // does not, and the sign and transaction tests below all require the
         // origin to still be authorized.
         await popup.check("#approve-remember");
-        await popup.click("#btn-approve");
+        await clickAndClose(popup, "#btn-approve");
 
         outcome = await settleRequest(env.dapp, "accounts");
     } finally {
@@ -2350,7 +2747,7 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env
     ]);
     const popup = await waitForApprovalWindow(env.ctx);
     await visible(popup, "#view-approve-sign");
-    await popup.click("#btn-reject-sign");
+    await clickAndClose(popup, "#btn-reject-sign");
 
     await assertUserRejection(
         env.dapp,
@@ -2453,7 +2850,7 @@ test("eth_signTypedData_v4 rejected returns a rejection to the page (#183)", asy
     ]);
     const popup = await waitForApprovalWindow(env.ctx);
     await visible(popup, "#view-approve-sign");
-    await popup.click("#btn-reject-sign");
+    await clickAndClose(popup, "#btn-reject-sign");
 
     await assertUserRejection(
         env.dapp,
@@ -2611,7 +3008,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
     ]);
     const popup = await waitForApprovalWindow(env.ctx);
     await visible(popup, "#view-approve-tx");
-    await popup.click("#btn-reject-tx");
+    await clickAndClose(popup, "#btn-reject-tx");
 
     await assertUserRejection(
         env.dapp,
@@ -2729,6 +3126,11 @@ async function main() {
         // The recovery phrase of the wallet created in test 2, so later
         // tests can assert on the real secret rather than its shape.
         phrase: null,
+        // What the Settings section (#229) actually observed. A guard test
+        // at the end of that section demands the full set, so a skipped or
+        // silently shortened assertion reddens the run instead of shrinking
+        // it.
+        settingsCoverage: new Set(),
         // Confirmation-screen heights, measured in the pending state and
         // compared against every later state of the same screen.
         ethPendingHeight: null,

tests/popupElementIds.test.js

@@ -0,0 +1,182 @@
+// Every element id the popup views look up must exist in the markup they
+// look it up in.
+//
+// The failure this catches: `$("settings-hide-dsut")` is valid JavaScript
+// referring to a defined function, so neither jest (node environment, no
+// DOM) nor a linter has anything to object to. At runtime `$()` returns
+// null and the next property access throws, which in `init()` aborts the
+// rest of that view's wiring and takes the whole screen down. Settings is
+// the densest concentration of these lookups in the codebase.
+//
+// This is the cheap general half of the guard: it runs in `make check`
+// with no browser and covers every id in every view, not the ones some
+// test happens to click. The expensive specific half is the Settings
+// section of the end-to-end suite (tests/e2e/run.js), which proves the
+// screen actually comes up and its controls work.
+//
+// Scope and limits, stated rather than implied:
+//   - Only literal string arguments are resolvable statically. A call
+//     like `$(containerId)` is invisible here; those are covered by the
+//     e2e run instead.
+//   - `document.getElementById()` is checked too, minus the ids listed in
+//     RUNTIME_CREATED_IDS, which name nodes the code creates itself and
+//     which are legitimately absent from the static markup.
+
+"use strict";
+
+const fs = require("fs");
+const path = require("path");
+
+const POPUP_DIR = path.join(__dirname, "..", "src", "popup");
+const POPUP_HTML_PATH = path.join(POPUP_DIR, "index.html");
+
+// Nodes built at runtime rather than authored in index.html. Each one must
+// be created unconditionally by the code before it is ever looked up.
+const RUNTIME_CREATED_IDS = new Set([
+    // Created by updateDebugBanner() in src/popup/views/helpers.js.
+    "debug-banner",
+]);
+
+// Every id lookup the popup performs with a literal argument, as
+// {id, file, line, source} records.
+//
+// showView("x") is included because it resolves to the element id
+// "view-x": a view name with no matching section is the same defect one
+// indirection further out.
+const PATTERNS = [
+    { re: /\$\(\s*"([^"\n]+)"\s*\)/g, id: (m) => m[1], source: "$()" },
+    {
+        re: /document\.getElementById\(\s*"([^"\n]+)"\s*\)/g,
+        id: (m) => m[1],
+        source: "getElementById()",
+    },
+    {
+        re: /\b(?:showError|hideError)\(\s*"([^"\n]+)"/g,
+        id: (m) => m[1],
+        source: "showError()/hideError()",
+    },
+    {
+        re: /\bshowView\(\s*"([^"\n]+)"\s*\)/g,
+        id: (m) => "view-" + m[1],
+        source: "showView()",
+    },
+];
+
+function jsFilesUnder(dir) {
+    const out = [];
+    for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+        const full = path.join(dir, entry.name);
+        if (entry.isDirectory()) {
+            out.push(...jsFilesUnder(full));
+        } else if (entry.name.endsWith(".js")) {
+            out.push(full);
+        }
+    }
+    return out.sort();
+}
+
+function lineOf(text, index) {
+    return text.slice(0, index).split("\n").length;
+}
+
+function collectReferences() {
+    const refs = [];
+    for (const file of jsFilesUnder(POPUP_DIR)) {
+        const text = fs.readFileSync(file, "utf8");
+        const rel = path.relative(path.join(__dirname, ".."), file);
+        for (const { re, id, source } of PATTERNS) {
+            re.lastIndex = 0;
+            let m;
+            while ((m = re.exec(text)) !== null) {
+                refs.push({
+                    id: id(m),
+                    file: rel,
+                    line: lineOf(text, m.index),
+                    source,
+                });
+            }
+        }
+    }
+    return refs;
+}
+
+function collectHtmlIds(html) {
+    const ids = [];
+    const re = /\bid="([^"]+)"/g;
+    let m;
+    while ((m = re.exec(html)) !== null) ids.push(m[1]);
+    return ids;
+}
+
+const HTML = fs.readFileSync(POPUP_HTML_PATH, "utf8");
+const HTML_IDS = collectHtmlIds(HTML);
+const HTML_ID_SET = new Set(HTML_IDS);
+const REFERENCES = collectReferences();
+
+describe("every element id the popup looks up exists in its markup", () => {
+    // A guard that found nothing to check would pass forever. If a
+    // refactor renames the directory, changes the helper, or moves the
+    // markup, this fails instead of quietly covering zero call sites.
+    // The floors are far below the counts measured when this was written
+    // (434 lookups across 20 of the 24 files under src/popup/, against 274
+    // ids in the markup), so ordinary churn does not trip them.
+    test("the scan actually found the code and the markup", () => {
+        const files = new Set(REFERENCES.map((r) => r.file));
+        expect(files.size).toBeGreaterThanOrEqual(15);
+        expect(REFERENCES.length).toBeGreaterThanOrEqual(300);
+        expect(HTML_IDS.length).toBeGreaterThanOrEqual(200);
+
+        // The densest screen, named explicitly: a scan that stopped
+        // covering src/popup/views/settings.js is the exact regression
+        // this file was written for.
+        expect(
+            files.has(path.join("src", "popup", "views", "settings.js")),
+        ).toBe(true);
+        expect(
+            REFERENCES.some((r) => r.id === "settings-hide-spoofed-symbols"),
+        ).toBe(true);
+        expect(REFERENCES.some((r) => r.id === "view-settings")).toBe(true);
+    });
+
+    test("no lookup names an id that src/popup/index.html does not define", () => {
+        const missing = REFERENCES.filter(
+            (r) => !HTML_ID_SET.has(r.id) && !RUNTIME_CREATED_IDS.has(r.id),
+        ).map(
+            (r) =>
+                r.file +
+                ":" +
+                r.line +
+                " " +
+                r.source +
+                ' looks up id "' +
+                r.id +
+                '", which is not in src/popup/index.html',
+        );
+
+        expect(missing).toEqual([]);
+    });
+
+    test("every id excused as runtime-created is still looked up somewhere", () => {
+        // Otherwise the exception list becomes a place stale names
+        // accumulate, and the next real miss can be waved through by
+        // adding one more.
+        for (const id of RUNTIME_CREATED_IDS) {
+            expect(REFERENCES.some((r) => r.id === id)).toBe(true);
+            expect(HTML_ID_SET.has(id)).toBe(false);
+        }
+    });
+
+    test("index.html defines no id twice", () => {
+        // getElementById returns the first match, so a duplicate id means
+        // one of the two elements can never be reached by the code that
+        // thinks it owns it.
+        const seen = new Set();
+        const duplicated = [];
+        for (const id of HTML_IDS) {
+            if (seen.has(id)) duplicated.push(id);
+            seen.add(id);
+        }
+
+        expect(duplicated).toEqual([]);
+    });
+});

tests/symbolSpoof.test.js

@@ -384,7 +384,7 @@ describe("the shipped token list", () => {
                 "0xab5eb14c09d416f0ac63661e57edb7aecdb9befa", // Metronome Synth USD
             ],
             MUSD: [
-                "0xaca92e438df0b2401ff60da7e4337b687a2435da", // MetaMask USD
+                "0xaca92e438df0b2401ff60da7e4337b687a2435da",
                 "0xdd468a1ddc392dcdbef6db6e34e89aa338f9f186", // Mezo USD
             ],
             JPYC: [