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
@@ -1807,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

@@ -81,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,
@@ -248,6 +249,18 @@ but the review is broader than any of them.
   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

@@ -489,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) {
@@ -505,7 +545,6 @@ runtime.onConnect.addListener((port) => {
                 }
                 settleApproval(id, { approved: false, remember: false });
             }
-            resetPopupUrl();
         });
     }
 });
@@ -1060,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,
@@ -1113,19 +1163,17 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
     }
 
     // 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("");
-        if (!sender.url || !sender.url.startsWith(extUrl)) {
         sendResponse({ error: "Unauthorized sender" });
         return false;
     }
-    }
 
     if (msg.type === "AUTISTMASK_GET_APPROVAL") {
         const approval = pendingApprovals[msg.id];
@@ -1156,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;

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 () => {

tests/backgroundApproval.test.js

@@ -39,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 = {
@@ -170,8 +185,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: {
@@ -190,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: {
@@ -218,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");
@@ -286,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) {
@@ -296,6 +397,8 @@ function loadBackground(options) {
         send,
         requestTx,
         requestSign,
+        requestSite,
+        connectApproval,
         closeWindow,
         broadcastTransaction,
         loadState,
@@ -1615,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

@@ -2184,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;
@@ -2258,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
@@ -2487,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,
@@ -2518,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 {
@@ -2673,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,
@@ -2776,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,
@@ -2934,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,

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: [