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

The blocklist URL in shipped code named a competitor and pointed at a moving
ref, and the extension re-fetched from it every 24 hours, which also meant a
third party decided what this wallet warns about. All of that is gone.

script/vendor-blocklist fetches upstream at a pinned commit, verifies the
sha256 of the bytes that commit serves, and writes
src/shared/phishingBlocklist.json. It is build-time tooling, never shipped, and
the one place in the repo that names the upstream project; a source reference
nobody can verify is not a source reference.

The artifact stores truncated sha256 digests rather than domain names. That is
what censors it: the previous file contained the competitor's name 6,475 times,
as phishing domains impersonating them, and not one of those domains is
dropped. It also makes lookups a binary search over a fixed-width string, so
nothing is built at module load — which matters on MV3, where the worker
re-evaluates the module on every wake — and takes the file from 8.7 MB to
1.7 MB.

script/check-censored enforces the rest: it reads the name out of the vendoring
script rather than repeating it, and fails on any occurrence in the working
tree or under dist/ that is not one of the two literals shipped code cannot
avoid. It runs in make check, which inspects dist/ when there is one and says
loudly when there is not, and again with --require-dist at the end of every
make build.

Removing the runtime fetch retires the delta, the extension-storage persistence
and the 24-hour alarm from #158. A retired alarm is now cleared rather than
left waking the worker forever on installs that already have it.

The e2e suite drives the warning end to end from a real blocklisted origin
served as a real http(s) site, with a control asserting the banner stays hidden
for one that is not listed. Its service-worker interception canary needed a new
anchor, since the startup fetch it used to watch for no longer happens: it now
wakes the worker with a message and asks it for one throwaway fetch.

LICENSE no longer cites a repository that returns 404.

eslint.config.js gains one block: script/lib/ holds node programs the shell
entrypoints call, and without it they lint with no globals at all.

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: the pinned source reference in `script/vendor-blocklist`, the two
-  provider-shim identifiers in `src/content/inpage.js`, and one ERC-20's
-  on-chain name in `src/shared/tokenList.js`. Each is scoped to that path and
-  fails anywhere else. Part of `make check`, which inspects `dist/` when there
-  is one and says loudly when there is not; `make build` re-runs it with
-  `--require-dist`, so a build artifact is always covered
+  exceptions, which are the pinned source reference in `script/vendor-blocklist`
+  and the two literals shipped code cannot avoid (the `isMetaMask`/`_metamask`
+  provider shim, and one ERC-20's on-chain name in the token list). Part of
+  `make check`, which inspects `dist/` when there is one and says loudly when
+  there is not; `make build` re-runs it with `--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
@@ -1807,14 +1807,12 @@ 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 definition and the only file that spells
-the name in prose: it is build-time tooling, never shipped, and it records the
-exact URL, the commit it is pinned to and the sha256 of the bytes that commit
-serves, because a source reference nobody can verify is not a source reference.
-`script/check-censored` reads the name back out of that one file and fails the
-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.
+`script/vendor-blocklist` is the single exception and the single definition: it
+is build-time tooling, never shipped, and it records the exact URL, the commit
+it is pinned to and the sha256 of the bytes that commit serves, because a source
+reference nobody can verify is not a source reference. `script/check-censored`
+reads the name back out of that one file and fails the build if it appears
+anywhere else.
 
 ## Author
 

TODO.md

@@ -81,13 +81,12 @@ 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, each permitted occurrence scoped to the one path allowed
-  to carry it; the name now appears only in the vendoring script, which defines
-  it once, in the provider-shim identifiers in `src/content/inpage.js`, and in
-  one ERC-20's on-chain name in `src/shared/tokenList.js`. Removing the fetch
-  retired the delta, the persistence and the 24-hour alarm from
-  [#158](https://git.eeqj.de/sneak/AutistMask/issues/158), and retired alarms
-  are now cleared rather than left running on existing installs. Two
+  end of every build; the name now appears only in the vendoring script, which
+  defines it once, and in the two literals shipped code cannot avoid (the
+  `isMetaMask`/`_metamask` provider shim, and one ERC-20's on-chain name).
+  Removing the fetch retired the delta, the persistence and the 24-hour alarm
+  from [#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,
@@ -228,39 +227,6 @@ 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,27 +9,25 @@
 #   - 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 two provider-shim identifiers in src/content/inpage.js. Protocol
-#     identifiers dApps feature-detect on; renaming them does not rename them in
-#     their code, it only stops this wallet working on their sites.
-#   - the on-chain name of the MUSD ERC-20 in src/shared/tokenList.js. It is not
-#     what backs symbol-spoof detection — that reads symbol and address — but
-#     the wallet already surfaces the on-chain name of any token the user holds
-#     (src/shared/balances.js), and this contract's on-chain name is that
-#     string, so censoring the repo cannot stop the wallet displaying it.
-#     Dropping the entry instead would cost the user MUSD spoof detection.
+#   - the literal `isMetaMask` and the `_metamask` object in
+#     src/content/inpage.js. A protocol identifier dApps feature-detect on;
+#     renaming it does not rename it in their code, it only stops this wallet
+#     working on their sites.
+#   - the literal `MetaMask USD` in src/shared/tokenList.js. The on-chain name
+#     of an ERC-20 the user may hold, recorded next to its address and symbol,
+#     which is what lets symbol-spoofing detection tell the real one from a
+#     forgery. Altering it would misidentify an asset to its owner.
 #
 # Everything else fails, in the working tree and under dist/. The last two are
-# literals rather than whole files, so they are enforced by counting, and each
-# literal is scoped to the path allowed to carry it: a file may contain the name
-# only as many times as it contains the literals permitted *there*, and zero
-# times anywhere else. The emitted bundles carry them too, so a plain "the name
-# must not appear in dist/" could never have passed.
+# literals rather than files, so they are enforced by counting: a file may
+# contain the name only as many times as it contains those exact strings. The
+# emitted bundles carry both, so a plain "the name must not appear in dist/"
+# could never have passed.
 #
-# The name itself is not written in this file. script/vendor-blocklist is the
-# one place in this repo that defines it, and this reads it back out of there —
-# so the repo-wide grep this check exists to enforce keeps returning exactly the
-# files named above, and this file is not one of them.
+# The name itself is not written here. script/vendor-blocklist is the one place
+# in this repo that defines it, and this reads it back out of there — so the
+# repo-wide grep this check exists to enforce keeps returning exactly the files
+# named above, and this file is not one of them.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -44,7 +42,7 @@ SCAN_FLAG="--scan-paths"
 
 VENDOR_SCRIPT="$ROOT/script/vendor-blocklist"
 
-# Set by extract_name / make_literals_file.
+# Set by extract_name / write_allowed_literals.
 NAME=""
 ALLOWED_LITERALS_FILE=""
 
@@ -81,32 +79,17 @@ extract_name() {
     esac
 }
 
-make_literals_file() {
+# Occurrences allowed anywhere, including in the emitted bundles. Each contains
+# 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."
-}
-
-# The literals $1 may carry, and nothing else may. Each contains the name
-# exactly once, which is what makes counting them sound; each is scoped to its
-# 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
+    {
+        echo "is$NAME"
+        echo "_$NAME"
+        echo "$NAME USD"
+    } >"$ALLOWED_LITERALS_FILE"
 }
 
 # How many times does $1 contain the name (TOTAL), and how many of those are one
@@ -131,14 +114,6 @@ 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=$?
@@ -156,26 +131,17 @@ count_matches() {
 # counting as everything else rather than a second copy of it.
 scan_paths() {
     for _file in "$@"; do
-        # dist/ arrives absolute (find) and the worktree relative (git
-        # 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 ;;
+        case "$_file" in
+        "$VENDOR_SCRIPT" | "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: $_rel: $TOTAL occurrence(s) of the name," \
-            "$ALLOWED of them allowed at this path" >&2
+        echo "check-censored: $_file: $TOTAL occurrence(s) of the name," \
+            "$ALLOWED of them allowed" >&2
         grep -a -n -i -F -e "$NAME" -- "$_file" | cut -c1-140 | head -5 >&2
     done
     [ "$FAILED" -eq 0 ]
@@ -244,7 +210,7 @@ main() {
     if [ "${1-}" = "$SCAN_FLAG" ]; then
         shift
         extract_name
-        make_literals_file
+        write_allowed_literals
         scan_paths "$@"
         return $?
     fi
@@ -257,7 +223,7 @@ main() {
     esac
 
     extract_name
-    make_literals_file
+    write_allowed_literals
 
     echo "Checking for censored names..."
 

src/background/index.js

@@ -165,16 +165,6 @@ 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 (
@@ -489,53 +479,13 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
     });
 }
 
-// Anything only the extension's own pages may say. A content script speaks
-// with the page's URL, so this is what separates the popup from the site the
-// popup is being asked about.
-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.
+// Detect when an approval popup (browser-action) closes without a response.
+// TX and sign approvals now use windows.create() and are handled by the
+// windows.onRemoved listener below, but we still handle site-connection
+// approval disconnects here.
 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) {
@@ -545,6 +495,7 @@ runtime.onConnect.addListener((port) => {
                 }
                 settleApproval(id, { approved: false, remember: false });
             }
+            resetPopupUrl();
         });
     }
 });
@@ -1099,21 +1050,10 @@ 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,
@@ -1139,41 +1079,26 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // keep fallback
             }
         }
-        handleRpc(msg.method, msg.params, trustedOrigin)
-            .then((response) => {
+        handleRpc(msg.method, msg.params, trustedOrigin).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) && !isExtensionSender(sender)) {
+    if (POPUP_ONLY_TYPES.includes(msg.type)) {
+        const extUrl = runtime.getURL("");
+        if (!sender.url || !sender.url.startsWith(extUrl)) {
             sendResponse({ error: "Unauthorized sender" });
             return false;
         }
+    }
 
     if (msg.type === "AUTISTMASK_GET_APPROVAL") {
         const approval = pendingApprovals[msg.id];
@@ -1204,6 +1129,15 @@ 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;
@@ -1254,10 +1188,6 @@ 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
@@ -1341,7 +1271,6 @@ 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(
@@ -1373,28 +1302,7 @@ 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;
     }
 
@@ -1478,25 +1386,7 @@ 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;
-    approvalPort = runtimeApi().connect({ name: "approval:" + id });
+    runtimeApi().connect({ name: "approval:" + id });
 
     let details = null;
     try {
@@ -476,14 +476,6 @@ 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
@@ -551,28 +543,6 @@ 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);
@@ -583,11 +553,25 @@ function init(_ctx) {
     });
 
     $("btn-approve").addEventListener("click", () => {
-        decideSite(true);
+        const remember = $("approve-remember").checked;
+        notify({
+            type: "AUTISTMASK_APPROVAL_RESPONSE",
+            id: approvalId,
+            approved: true,
+            remember,
+        });
+        window.close();
     });
 
     $("btn-reject").addEventListener("click", () => {
-        decideSite(false);
+        const remember = $("approve-remember").checked;
+        notify({
+            type: "AUTISTMASK_APPROVAL_RESPONSE",
+            id: approvalId,
+            approved: false,
+            remember,
+        });
+        window.close();
     });
 
     $("btn-approve-tx").addEventListener("click", async () => {

tests/backgroundApproval.test.js

@@ -20,11 +20,6 @@
 
 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 =
@@ -39,21 +34,6 @@ 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 = {
@@ -164,14 +144,6 @@ 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: [
@@ -185,21 +157,13 @@ 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(
-                    opts.storageGet ||
-                        (async () => ({ autistmask: persisted })),
-                ),
+                get: jest.fn(async () => ({ autistmask: persisted })),
                 set: jest.fn(async () => {}),
             },
         },
@@ -210,14 +174,7 @@ function loadBackground(options) {
                     messageListener = fn;
                 },
             },
-            // 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;
-                },
-            },
+            onConnect: { addListener: () => {} },
             lastError: null,
         },
         windows: {
@@ -245,17 +202,7 @@ function loadBackground(options) {
             query: (q, cb) => cb([]),
             sendMessage: () => {},
         },
-        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() } : {}),
-        },
+        action: { setPopup: () => {} },
     };
 
     require("../src/background/index");
@@ -323,70 +270,6 @@ 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) {
@@ -397,8 +280,6 @@ function loadBackground(options) {
         send,
         requestTx,
         requestSign,
-        requestSite,
-        connectApproval,
         closeWindow,
         broadcastTransaction,
         loadState,
@@ -425,15 +306,6 @@ 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();
@@ -1500,200 +1372,6 @@ 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();
@@ -1718,197 +1396,3 @@ 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

@@ -2184,13 +2184,32 @@ async function reserveApprovalTab(env) {
     // one down with it.
     env.approvalTab = await env.ctx.newPage();
 
-    // This tab runs the shipped popup with nothing patched. The site
-    // approval buttons decide and then close on the next line, and the two
-    // site-approval tests below are therefore the real-browser
-    // approve-then-immediate-close and reject-then-immediate-close cases: the
-    // decision rides the approval port, which also carries the disconnect the
-    // close causes, so it is delivered ahead of it and the outcome does not
-    // depend on the teardown timing (#275).
+    // The one accommodation this section makes to the shipped code, and the
+    // reason for it.
+    //
+    // Both approval buttons call runtime.sendMessage() and then window.close()
+    // on the next line. Closing this page disconnects the approval port, and
+    // the disconnect handler in src/background/index.js settles a pending
+    // site approval as a rejection. In a tab those two race and the teardown
+    // 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;
@@ -2239,95 +2258,6 @@ 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
@@ -2557,11 +2487,7 @@ 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");
-        // 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 popup.click("#btn-reject");
 
         await assertUserRejection(
             env.dapp,
@@ -2592,7 +2518,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 clickAndClose(popup, "#btn-approve");
+        await popup.click("#btn-approve");
 
         outcome = await settleRequest(env.dapp, "accounts");
     } finally {
@@ -2747,7 +2673,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 clickAndClose(popup, "#btn-reject-sign");
+    await popup.click("#btn-reject-sign");
 
     await assertUserRejection(
         env.dapp,
@@ -2850,7 +2776,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 clickAndClose(popup, "#btn-reject-sign");
+    await popup.click("#btn-reject-sign");
 
     await assertUserRejection(
         env.dapp,
@@ -3008,7 +2934,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
     ]);
     const popup = await waitForApprovalWindow(env.ctx);
     await visible(popup, "#view-approve-tx");
-    await clickAndClose(popup, "#btn-reject-tx");
+    await popup.click("#btn-reject-tx");
 
     await assertUserRejection(
         env.dapp,

tests/symbolSpoof.test.js

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