fix: each browser-suite run builds and removes an image of its own (closes #547)

script/test-e2e and script/test-e2e-firefox tagged their image with one
fixed name. When two clones ran a suite at once, the second build moved
that tag, the image store deleted the first run's now-untagged image, and
that suite exited 125 without running a test. Each run now tags its image
with the suite's name and twelve random hex digits and runs it by that
tag. The cleanup that runs on exit, Ctrl-C, kill and a closed terminal
removes the run's container if docker's client left it running, then the
image. The image-ID file that guarded against a moving tag is gone.
README.md's End-to-End Tests section says which endings clean up.

Model: opus-5-5

README.md

@@ -239,8 +239,12 @@ provide:
   `make check` stays non-mutating. Linting runs only in the container, because
   an ESLint result that depends on whichever ESLint the host happens to have is
   not a result, so docker is required to lint.
-- `script/fmt` — format all files (writes), on the host
-- `script/fmt-check` — check formatting (read-only), on the host
+- `script/fmt` — format all files (writes), on the host, with the `prettier`
+  that `script/bootstrap` installs in `node_modules/`; when that one is missing
+  it fails and says to run `script/bootstrap`, instead of using a `prettier`
+  from the `PATH`
+- `script/fmt-check` — check formatting (read-only), on the host, with the same
+  `prettier` and the same refusal
 - `script/check` — run `script/test`, `script/lint` and `script/fmt-check`
 - `script/check-censored` — assert the competitor name RULES.md bars appears
   nowhere in the working tree or under `dist/` outside its documented
@@ -343,6 +347,14 @@ are outside `make check`. Neither opens the popup from the toolbar button: both
 load its page in an ordinary tab, so what the toolbar popup itself adds, its
 size and its closing when it loses focus, is covered by neither.
 
+Each run builds its image under a tag of its own, `autistmask-e2e-chrome:` or
+`autistmask-e2e-firefox:` followed by twelve random hex digits, so two clones
+can run the same suite at once without one build taking the other's image away.
+The run removes that image, and its container if one is still running, after the
+suite passes or fails, on Ctrl-C, on `kill`, and when the terminal or SSH
+session it runs in closes. A run killed outright (`kill -9`) cannot clean up:
+its image stays under its tag until removed by hand with `docker image rm`.
+
 ### Chrome (`make test-e2e`)
 
 `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
@@ -1641,6 +1653,12 @@ view would leave a wallet one click from deletion.
       the contract's own answer is read at signing time only to be compared with
       it, and a disagreement is a refusal rather than a preference for either
       value (`src/shared/transferAmount.js`)
+    - "Sign & Send" after the active network was switched in Settings while this
+      screen was open → nothing is signed; "The active network changed after
+      this transaction was reviewed, so it was not sent. Please go back and
+      review it again." in the errors box, no screen change. The transaction
+      carries the chain id of the network the screen was shown for, so it cannot
+      be signed for another
     - "Sign & Send" (wrong password) → "Wrong password." on the password error
       line, no screen change
     - "Back" → **Send**
@@ -2071,7 +2089,9 @@ view would leave a wallet one click from deletion.
   artifact can be compared with it field for field. A nonce the site supplies is
   ignored: the nonce is always the account's next nonce from the node, so a site
   cannot replace one of the user's pending transactions or leave this one stuck
-  behind a gap. A request that cannot be populated — unreachable node, reverting
+  behind a gap. A gas limit the site supplies, as `gas` or `gasLimit`, is used
+  in place of the node's estimate; a request giving both with different values
+  is refused. A request that cannot be populated — unreachable node, reverting
   gas estimate — opens no window and is failed back to the site. Only one
   transaction approval exists at a time: populating fixes the nonce, so a second
   `eth_sendTransaction` arriving while one is unanswered is refused with
@@ -2124,7 +2144,12 @@ view would leave a wallet one click from deletion.
       blocklist)
     - Site origin (bold, scheme and port included) + "wants you to sign a
       message"
-    - Danger warning box (shown for `eth_sign`, which signs a raw hash)
+    - Warning box, shown for `eth_sign`: "This site used the older eth_sign
+      method. The data will be signed as a personal message." An `eth_sign`
+      request names the address first and the data second. Its data is signed
+      exactly as a `personal_sign` message is, as an EIP-191 personal message,
+      which is what the Ethereum JSON-RPC specification defines `eth_sign` to
+      return.
     - Type: "Personal message" or "Typed data (EIP-712)"
     - From: color dot + full address + etherscan link
     - Message: for `personal_sign` and `eth_sign`, the text the message's bytes
@@ -2139,7 +2164,12 @@ view would leave a wallet one click from deletion.
       Bytes that are not UTF-8 are shown as "This message is not text." For
       typed data, formatted domain/type/message fields (EIP-712). The primary
       type shown is the one ethers signs, derived from the typed data's `types`,
-      not the type the site states.
+      not the type the site states. The message shows only the fields that type
+      declares, at every level of nesting and in the type's order: ethers signs
+      those and drops any other key. Typed data carrying other keys is not
+      refused; each such key is listed after the message under "Not part of the
+      signature", by its place in the message (`details.note`,
+      `details[1].note`) and its value.
     - Token permission warning, at the top of the message (typed data whose
       primary type is `Permit`, as in EIP-2612, or one of Permit2's signature
       types): "⚠️ TOKEN PERMISSION: Signing this lets the spender below take the
@@ -2160,6 +2190,13 @@ view would leave a wallet one click from deletion.
     - Typed data that states no primary type, or one other than the type it
       would be signed as, or that cannot be read → shown with the error line
       saying so and "Sign" disabled; only "Reject" remains
+    - Typed data whose domain `chainId` names a chain other than the active
+      network's → shown with the error line naming both chain ids and "Sign"
+      disabled; only "Reject" remains. EIP-712 asks for this, because the
+      signature is valid on the chain the domain names. Typed data with no
+      `chainId` is not refused. The background refuses such a signature too,
+      against the network active when the signature reaches it, and answers the
+      site with the same sentence
     - A `personal_sign` or `eth_sign` message that is not hex (`0x` or `0X` and
       an even number of hex digits, the form ethers' `getBytes` reads when
       signing) → shown as plain text, with the error line "This message is plain
@@ -2652,7 +2689,8 @@ plaintext list of anyone's domain names.
 When a dApp on a blocklisted domain requests a wallet connection, transaction
 approval, or signature, the approval popup displays a prominent red warning
 banner alerting the user. The domain checker matches exact hostnames and all
-parent domains (subdomain matching).
+parent domains (subdomain matching), and checks a hostname written with a
+trailing dot (`name.`) as `name`.
 
 #### Transaction Decoding
 

script/fmt

@@ -25,6 +25,13 @@ run_yarn() {
 
 main() {
     cd "$ROOT"
+    # Without the repo's own prettier, yarn runs whatever prettier is on
+    # the PATH, which may be a version that formats files differently.
+    if [ ! -x node_modules/.bin/prettier ]; then
+        echo "fmt: this repo's prettier is not installed in node_modules/," \
+            "so run script/bootstrap first." >&2
+        exit 1
+    fi
     echo "Formatting..."
     run_yarn run fmt
 }

script/fmt-check

@@ -26,6 +26,13 @@ run_yarn() {
 
 main() {
     cd "$ROOT"
+    # Without the repo's own prettier, yarn runs whatever prettier is on
+    # the PATH, which may be a version that formats files differently.
+    if [ ! -x node_modules/.bin/prettier ]; then
+        echo "fmt-check: this repo's prettier is not installed in" \
+            "node_modules/, so run script/bootstrap first." >&2
+        exit 1
+    fi
     echo "Checking formatting..."
     run_yarn run fmt-check
 }

script/test-e2e

@@ -22,13 +22,29 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
-
-IIDFILE=""
+# Each run builds, runs and then removes an image under a tag of its own.
+# Under one tag shared by every run, a second clone's build moves the tag
+# off this run's image, and the image store can then delete that untagged
+# image before or while this run uses it. The tag is random rather than
+# $$ because CI jobs share one docker daemon, but each job's container
+# numbers its processes from scratch.
+SUFFIX="$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+NAME="$("$SCRIPT_DIR/projectname")-e2e-chrome"
+IMAGE="$NAME:$SUFFIX"
+# Both docker runs below use this container name: docker run --rm returns
+# only once its container is removed, so the second never meets the first.
+CONTAINER="$NAME-$SUFFIX"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # After a third Ctrl-C docker's client exits whether or not the
+    # container has stopped, and a container still running keeps the image
+    # in use.
+    if docker container inspect "$CONTAINER" >/dev/null 2>&1; then
+        docker container rm -f "$CONTAINER" >/dev/null
+    fi
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -40,22 +56,16 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    trap 'cleanup; exit 130' INT TERM
+    # HUP: the terminal or SSH session running the suite closed.
+    trap 'cleanup; exit 130' HUP INT TERM
 
     echo "Building the Chrome e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/Dockerfile .
 
     echo "Running e2e suite in the pinned Playwright container..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --ipc=host: Chromium's shared-memory needs more than the default
     #   64MB /dev/shm or renderers crash.
     # HOME=/tmp: the image's root home is not a reliable place for the
@@ -75,11 +85,12 @@ main() {
     #   probe. The image is pinned by digest, so this can only ever bite
     #   on a deliberate bump.
     docker run --rm \
+        --name "$CONTAINER" \
         --ipc=host \
         -e HOME=/tmp \
         -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
         -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/run.js
 
     # Where chrome.storage.local lives, and what moves it: two unpacked loads
@@ -93,9 +104,10 @@ main() {
     # --host-resolver-rules=MAP * ~NOTFOUND so nothing it does can leave.
     echo "Running the extension-id and storage-partition observations..."
     docker run --rm \
+        --name "$CONTAINER" \
         --ipc=host \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/storagePartition.js
 }
 

script/test-e2e-firefox

@@ -21,13 +21,23 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
-
-IIDFILE=""
+# A tag and a container name of this run's own, as in script/test-e2e,
+# which says why.
+SUFFIX="$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+NAME="$("$SCRIPT_DIR/projectname")-e2e-firefox"
+IMAGE="$NAME:$SUFFIX"
+CONTAINER="$NAME-$SUFFIX"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # After a third Ctrl-C docker's client exits whether or not the
+    # container has stopped, and a container still running keeps the image
+    # in use.
+    if docker container inspect "$CONTAINER" >/dev/null 2>&1; then
+        docker container rm -f "$CONTAINER" >/dev/null
+    fi
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -39,22 +49,16 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    trap 'cleanup; exit 130' INT TERM
+    # HUP: the terminal or SSH session running the suite closed.
+    trap 'cleanup; exit 130' HUP INT TERM
 
     echo "Building the pinned Firefox e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/firefox/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/firefox/Dockerfile .
 
     echo "Running the Firefox e2e suite..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
     # --network none: the suite stubs nothing, so this is what keeps the
     #   run offline and deterministic. The extension swallows its own
@@ -69,10 +73,11 @@ main() {
     # "CanCreateUserNamespace() clone() failure: EPERM" on startup here;
     # it is cosmetic and headless Firefox runs fine without it.
     docker run --rm \
+        --name "$CONTAINER" \
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/run.js dist/firefox
 
     # The install/uninstall/re-install property, against the packaged XPI
@@ -82,10 +87,11 @@ main() {
     # and the suite above shares one session throughout.
     echo "Running the Firefox re-install suite against the packaged XPI..."
     docker run --rm \
+        --name "$CONTAINER" \
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/reinstall.js
 }
 

src/background/index.js

@@ -22,6 +22,7 @@ const { debugFetch, log } = require("../shared/log");
 const {
     verifySignedTx,
     verifySignature,
+    typedDataChainRefusal,
     failureIsRetryable,
     describeTxFailure,
     sameAddress,
@@ -632,8 +633,9 @@ function isExtensionSender(sender) {
 // 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.
+// disconnect. When their window closes, the windows.onRemoved listener below
+// rejects them, unless an approved attempt is still running, in which case
+// that attempt answers the page.
 runtime.onConnect.addListener((port) => {
     if (port.name.startsWith("approval:")) {
         const id = port.name.split(":")[1];
@@ -661,7 +663,9 @@ runtime.onConnect.addListener((port) => {
             const approval = pendingApprovals[id];
             if (approval) {
                 if (approval.type === "tx" || approval.type === "sign") {
-                    // Keep pending — user can reopen the toolbar popup
+                    // When its window closes, windows.onRemoved rejects it,
+                    // unless an approved attempt is still running, in which
+                    // case that attempt answers the page.
                     return;
                 }
                 settleApproval(id, { approved: false, remember: false });
@@ -984,9 +988,8 @@ async function handleRpc(method, params, origin) {
 
         if (method === "eth_sign") {
             signParams.dangerWarning =
-                "\u26a0\ufe0f DANGER: This site is requesting to sign a raw hash. " +
-                "This can be used to sign transactions that drain your funds. " +
-                "Only proceed if you fully understand what you are signing.";
+                "This site used the older eth_sign method. " +
+                "The data will be signed as a personal message.";
         }
 
         if (findPendingApproval(origin, "sign")) {
@@ -1764,7 +1767,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
 
         (async () => {
             try {
-                const activeAddress = await getActiveAddress();
+                const s = await getState();
+                const activeAddress = activeAddressOf(s);
                 // Same as the transaction path: the address the approval named
                 // is the one that must have signed, and a switch since then is
                 // a refusal rather than a signature from another account.
@@ -1783,6 +1787,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     signature,
                     approval.approvedFrom,
                 );
+                // The sign screen refuses typed data whose domain names
+                // another chain. A signature for it that arrives anyway is
+                // refused here, against the network active now.
+                const sp = approval.signParams;
+                if (
+                    sp.method === "eth_signTypedData_v4" ||
+                    sp.method === "eth_signTypedData"
+                ) {
+                    const refusal = typedDataChainRefusal(
+                        JSON.parse(sp.typedData).domain,
+                        networkById(s.networkId),
+                    );
+                    if (refusal) throw new ApprovalMismatchError(refusal);
+                }
                 settleApproval(msg.id, { signature }, { holdsClaim: true });
                 sendResponse({ signature });
             } catch (e) {

src/popup/views/approval.js

@@ -12,7 +12,7 @@ const {
     formatFee,
     tokenLabel,
 } = require("./helpers");
-const { state, saveState } = require("../../shared/state");
+const { state, saveState, currentNetwork } = require("../../shared/state");
 const {
     networkById,
     networkByChainId,
@@ -46,7 +46,10 @@ const {
 const { decryptWithPassword } = require("../../shared/vault");
 const { getSignerForAddress } = require("../../shared/wallet");
 const { walletDefect } = require("../../shared/walletDefects");
-const { describeSigningFailure } = require("../../shared/approvalVerify");
+const {
+    describeSigningFailure,
+    typedDataChainRefusal,
+} = require("../../shared/approvalVerify");
 const txStatus = require("./txStatus");
 const uniswap = require("../../shared/uniswap");
 const { notify, runtimeApi, sendMessage } = require("../../shared/browserApi");
@@ -443,7 +446,8 @@ function signedPrimaryType(types) {
 // Why a signature request cannot be signed, as a sentence for the error line,
 // or null when it can. Only typed data is refused: the `primaryType` the page
 // states has to be the type ethers will sign, or this screen would name one
-// message while another is signed.
+// message while another is signed. Its domain may name no chain but the active
+// network's.
 function typedDataRefusal(sp) {
     if (sp.method === "personal_sign" || sp.method === "eth_sign") return null;
     let data;
@@ -466,7 +470,7 @@ function typedDataRefusal(sp) {
             ", so it cannot be signed."
         );
     }
-    return null;
+    return typedDataChainRefusal(data.domain, currentNetwork());
 }
 
 // The largest amount a Permit2 allowance can hold, a uint160. Permit2 treats
@@ -622,10 +626,38 @@ function permitWarningHtml(types, primaryType, domain, message) {
     return html;
 }
 
+// What ethers signs of a message value of type `type`: a struct keeps only
+// the fields its type declares, in the type's order, and a list is read item
+// by item. ethers drops every other key, so each one is added to `notSigned`
+// with its place in the message and its value, for the screen to list apart.
+function signedValue(types, type, value, path, notSigned) {
+    const itemType = type.replace(/\[\d*\]$/, "");
+    if (itemType !== type) {
+        if (!Array.isArray(value)) return value;
+        return value.map((item, i) =>
+            signedValue(types, itemType, item, `${path}[${i}]`, notSigned),
+        );
+    }
+    const fields = types[type];
+    if (!fields || !value || typeof value !== "object") return value;
+    const at = (name) => (path ? `${path}.${name}` : name);
+    for (const [key, val] of Object.entries(value)) {
+        if (!fields.some((f) => f.name === key)) notSigned.push([at(key), val]);
+    }
+    return Object.fromEntries(
+        fields.map((f) => [
+            f.name,
+            signedValue(types, f.type, value[f.name], at(f.name), notSigned),
+        ]),
+    );
+}
+
 // The typed data as the screen shows it. The primary type shown is the one
 // ethers signs, never the page's word for it; typedDataRefusal() keeps the two
-// from differing on anything that can be signed. Only typed data that cannot
-// be read at all is shown as raw text, and typedDataRefusal() refuses it.
+// from differing on anything that can be signed. The Message section shows
+// only what is signed, and any other key the page sent is listed after it as
+// not part of the signature. Only typed data that cannot be read at all is
+// shown as raw text, and typedDataRefusal() refuses it.
 function formatTypedDataHtml(jsonStr) {
     let data;
     let primaryType;
@@ -659,12 +691,30 @@ function formatTypedDataHtml(jsonStr) {
     html += `<div class="mb-2"><div class="text-muted">Primary type</div>`;
     html += `<div class="font-bold">${escapeHtml(primaryType)}</div></div>`;
 
+    const lineHtml = (key, val) =>
+        `<div><span class="text-muted">${escapeHtml(key)}:</span> <span class="break-all">${escapeHtml(display(val))}</span></div>`;
+
     if (data.message) {
+        const notSigned = [];
+        const message = signedValue(
+            data.types,
+            primaryType,
+            data.message,
+            "",
+            notSigned,
+        );
         html += `<div class="mb-2"><div class="text-muted">Message</div>`;
-        for (const [key, val] of Object.entries(data.message)) {
-            html += `<div><span class="text-muted">${escapeHtml(key)}:</span> <span class="break-all">${escapeHtml(display(val))}</span></div>`;
+        for (const [key, val] of Object.entries(message)) {
+            html += lineHtml(key, val);
         }
         html += `</div>`;
+        if (notSigned.length > 0) {
+            html += `<div class="mb-2"><div class="text-muted">Not part of the signature</div>`;
+            for (const [key, val] of notSigned) {
+                html += lineHtml(key, val);
+            }
+            html += `</div>`;
+        }
     }
 
     return html;
@@ -721,7 +771,7 @@ function showSignApproval(details) {
             "This message is plain text, not hex, so it cannot be signed.";
     }
 
-    // Display danger warning for eth_sign (raw hash signing)
+    // The warning the background attaches to an eth_sign request.
     const warningEl = $("approve-sign-danger-warning");
     if (warningEl) {
         if (sp.dangerWarning) {

src/popup/views/confirmTx.js

@@ -101,6 +101,10 @@ function show(txInfo) {
     feeStatus = FEE_PENDING;
     feeWei = null;
     maxSendFees = null;
+    // The network this screen is drawn for, and the only one it sends on:
+    // Settings can switch the active network while the screen is open. The
+    // wait, success and error screens label the amount by it too.
+    txInfo.chainId = currentNetwork().chainId;
 
     const isErc20 = txInfo.token !== "ETH";
     // The raw symbol is the price-table key; the capped one is what the
@@ -490,7 +494,12 @@ async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
         );
         request = await contract.transfer.populateTransaction(tx.to, amount);
     }
-    const populated = await connectedSigner.populateTransaction(request);
+    // The chain id of the network the screen was drawn for: ethers refuses to
+    // populate it for a provider on any other network.
+    const populated = await connectedSigner.populateTransaction({
+        ...request,
+        chainId: tx.chainId,
+    });
     assertWithinCeilings(populated);
     return connectedSigner.sendTransaction(populated);
 }
@@ -547,6 +556,17 @@ function init(_ctx) {
             return;
         }
 
+        // Back from Settings only unhides this screen, still drawn for the
+        // network that was active when it was shown.
+        if (currentNetwork().chainId !== pendingTx.chainId) {
+            showSendError(
+                "The active network changed after this transaction was" +
+                    " reviewed, so it was not sent. Please go back and review" +
+                    " it again.",
+            );
+            return;
+        }
+
         const password = $("confirm-tx-password").value;
         if (!password) {
             showError(
@@ -575,10 +595,6 @@ function init(_ctx) {
         $("btn-confirm-send").disabled = true;
         $("btn-confirm-send").classList.add("text-muted");
 
-        // The network it is sent on. The wait, success and error screens
-        // label its amount by this, not by the network active when they draw.
-        pendingTx.chainId = currentNetwork().chainId;
-
         let tx;
         try {
             const signer = getSignerForAddress(

src/popup/views/transactionDetail.js

@@ -183,7 +183,7 @@ function render() {
         if (el) el.classList.add("hidden");
     }
 
-    loadFullTxDetails(tx.hash, tx.to, tx.chainId);
+    loadFullTxDetails(tx.hash, tx.chainId);
 
     const isoStr = isoDate(tx.timestamp);
     $("tx-detail-time").innerHTML =
@@ -291,7 +291,7 @@ function populateOnChainDetails(txData, chainId) {
     }
 }
 
-async function loadFullTxDetails(txHash, toAddress, chainId) {
+async function loadFullTxDetails(txHash, chainId) {
     const section = $("tx-detail-calldata-section");
     const actionEl = $("tx-detail-calldata-action");
     const detailsEl = $("tx-detail-calldata-details");
@@ -313,7 +313,10 @@ async function loadFullTxDetails(txHash, toAddress, chainId) {
         const inputData = txData.raw_input || txData.input || null;
         if (!inputData || inputData === "0x") return;
 
-        const decoded = decodeCalldata(inputData, toAddress || "");
+        // Decoded against the address the transaction was sent to, as the
+        // explorer reports it, not the history entry's `to`: for a token
+        // transfer that is the transfer's recipient, not the token contract.
+        const decoded = decodeCalldata(inputData, txData.to?.hash || "");
         if (decoded) {
             // Render decoded calldata matching approval view style
             actionEl.textContent = decoded.name;

src/shared/approvalTx.js

@@ -1,13 +1,14 @@
 // Preparation of the transaction an approval screen displays.
 //
-// A dApp's eth_sendTransaction normally fixes only `to`, `value` and `data`.
-// The nonce, the gas limit and the fees have to be filled in from the network
-// before anything can be signed, and whoever fills them in decides what the
-// user is shown. That work used to happen in the popup, after the user had
-// already approved: the numbers on the approval screen came from the popup and
-// were compared against nothing, so a compromised popup could display one fee
-// and sign another, and the ceilings in approvalVerify.js were all that stood
-// between the user and a fee that hands the validator the balance.
+// A dApp's eth_sendTransaction normally fixes `to`, `value` and `data`, and
+// sometimes the gas limit. The nonce, the fees, and the gas limit when the
+// site gives none have to be filled in from the network before anything can
+// be signed, and whoever fills them in decides what the user is shown. That
+// work used to happen in the popup, after the user had already approved: the
+// numbers on the approval screen came from the popup and were compared against
+// nothing, so a compromised popup could display one fee and sign another, and
+// the ceilings in approvalVerify.js were all that stood between the user and a
+// fee that hands the validator the balance.
 //
 // So it happens here instead, in the background, before the approval window is
 // opened. The background populates the transaction, shows that object, and
@@ -31,6 +32,7 @@ const {
     VoidSigner,
     accessListify,
     getAddress,
+    getBigInt,
     getBytes,
     hexlify,
     toQuantity,
@@ -47,10 +49,11 @@ const {
 // minutes long, which is not a wait anyone will sit through.
 const POPULATE_TIMEOUT_MS = 20000;
 
-// The request fields taken from the page. Anything else is dropped rather than
-// passed to ethers: the object is page-controlled, and a future ethers that
-// learns to carry a new transaction field must not start picking one up out of
-// it without this module knowing.
+// The request fields taken from the page, plus `gas`, which requestFrom()
+// takes as the gas limit. Anything else is dropped rather than passed to
+// ethers: the object is page-controlled, and a future ethers that learns to
+// carry a new transaction field must not start picking one up out of it
+// without this module knowing.
 //
 // The nonce is not taken from the page; it is always the account's next nonce
 // from the network. A page that chose it could replace one of the user's
@@ -84,6 +87,16 @@ function present(v) {
     return v !== null && v !== undefined && v !== "";
 }
 
+// Whether two quantities from the page are the same number. One that is not a
+// number matches nothing.
+function sameQuantity(a, b) {
+    try {
+        return getBigInt(a) === getBigInt(b);
+    } catch {
+        return false;
+    }
+}
+
 // These strings reach the user through the requesting page, so they are full
 // sentences even when the tail of one came from ethers or from the node.
 function sentence(text) {
@@ -111,6 +124,20 @@ function requestFrom(txParams, from) {
     for (const key of REQUEST_FIELDS) {
         if (present(txParams[key])) request[key] = txParams[key];
     }
+    // JSON-RPC calls the gas limit `gas`, and that is the name sites send.
+    // ethers only reads `gasLimit`: left as `gas` it would be dropped and an
+    // estimate signed in its place.
+    if (present(txParams.gas)) {
+        if (
+            present(request.gasLimit) &&
+            !sameQuantity(request.gasLimit, txParams.gas)
+        ) {
+            throw fail(
+                "The site asked for two different gas limits for this transaction.",
+            );
+        }
+        request.gasLimit = txParams.gas;
+    }
     if (
         present(request.type) &&
         !ALLOWED_TX_TYPES.includes(Number(request.type))

src/shared/approvalVerify.js

@@ -53,6 +53,7 @@ const {
     accessListify,
     formatEther,
     getAddress,
+    getBigInt,
     getBytes,
     toQuantity,
     verifyMessage,
@@ -635,6 +636,33 @@ function verifySignature(signParams, signature, expectedFrom) {
     return recovered;
 }
 
+// Why typed data cannot be signed while `network` is the active network, as a
+// sentence, or null when it can. EIP-712 has a wallet refuse typed data whose
+// domain names another chain, because the signature is valid on that chain.
+// ethers signs no chain id when the domain's is null or absent, and reads any
+// other with getBigInt(), so 1, "1" and "0x1" are the same chain here too.
+function typedDataChainRefusal(domain, network) {
+    const chainId = domain?.chainId;
+    if (chainId === undefined || chainId === null) return null;
+    let named;
+    try {
+        named = getBigInt(chainId);
+    } catch {
+        return "This typed data cannot be read, so it cannot be signed.";
+    }
+    const active = getBigInt(network.chainId);
+    if (named === active) return null;
+    return (
+        "This typed data is for chain id " +
+        named +
+        ", but the active network is " +
+        network.name +
+        " (chain id " +
+        active +
+        "), so it cannot be signed."
+    );
+}
+
 // The stage a transaction approval failed at. Which stage it is decides
 // whether the approval survives the failure.
 const TX_STAGE_SIGN = "sign";
@@ -796,6 +824,7 @@ function describeSigningFailure(response, fallbackMessage) {
 module.exports = {
     verifySignedTx,
     verifySignature,
+    typedDataChainRefusal,
     assertNoForbiddenFields,
     assertNothingUnchecked,
     assertCanonicalBytes,

src/shared/phishingDomains.js

@@ -110,11 +110,14 @@ function hashListed(hash) {
  * Generate hostname variants for subdomain matching.
  * "sub.evil.com" yields ["sub.evil.com", "evil.com"].
  *
+ * A trailing dot is dropped: "evil.com." is the fully qualified spelling of
+ * "evil.com", and the browser loads the same site from either.
+ *
  * @param {string} hostname
  * @returns {string[]}
  */
 function hostnameVariants(hostname) {
-    const h = hostname.toLowerCase();
+    const h = hostname.toLowerCase().replace(/\.$/, "");
     const variants = [h];
     const parts = h.split(".");
     // Parent domains: a.b.c.d -> b.c.d, c.d

src/shared/walletDefects.js

@@ -36,11 +36,13 @@ const DEFECTS = {
                 "derivation path beneath that key instead of from a master " +
                 "key, so the addresses listed here are not the ones that key " +
                 "produces under the standard path.",
-            "Signing and sending are disabled for this wallet. The addresses " +
-                "do descend from the extended private key you imported, so " +
-                "anything they hold is still reachable by software that " +
-                "repeats the same non-standard derivation. Check them in a " +
-                "block explorer before deciding what to do.",
+            "Signing, sending and private key export are disabled for this " +
+                "wallet. Anything these addresses hold can be reached outside " +
+                "this wallet with the extended private key you imported, if " +
+                "you kept a copy of it, or with the master extended private " +
+                "key or recovery phrase it came from, in software that " +
+                "repeats the same non-standard derivation. Check the " +
+                "addresses in a block explorer before deciding what to do.",
             "To see the addresses this key produces under the standard path, " +
                 "import the master extended private key, or the recovery " +
                 "phrase it came from, as a new wallet. Nothing here has been " +

tests/approvalTx.test.js

@@ -9,7 +9,7 @@
 // requesting page or the RPC node can say turns it into an approval that
 // should never have been raised.
 
-const { Network, Wallet } = require("ethers");
+const { Network, Transaction, Wallet, toQuantity } = require("ethers");
 const {
     prepareApprovalTx,
     serializeApprovedTx,
@@ -138,6 +138,43 @@ describe("prepareApprovalTx", () => {
         expect(approved.maxFeePerGas).toBe("0x12a05f200");
     });
 
+    // `gas` is the JSON-RPC name for the gas limit and the one sites send.
+    // The node would estimate 21000; the site's 200000 is what is shown and
+    // what is signed.
+    test("uses the gas limit a site sends as gas", async () => {
+        const approved = await prepareApprovalTx(
+            providerWith(),
+            signer.address,
+            { ...TX_PARAMS, gas: "0x30d40" },
+        );
+        expect(approved.gasLimit).toBe("0x30d40");
+        const signed = Transaction.from(
+            await signer.signTransaction({ ...approved }),
+        );
+        expect(signed.gasLimit).toBe(200000n);
+    });
+
+    test("accepts gas and gasLimit that name the same number", async () => {
+        const approved = await prepareApprovalTx(
+            providerWith(),
+            signer.address,
+            { ...TX_PARAMS, gas: "0x30d40", gasLimit: 200000 },
+        );
+        expect(approved.gasLimit).toBe("0x30d40");
+    });
+
+    test("refuses gas and gasLimit that disagree", async () => {
+        await expect(
+            prepareApprovalTx(providerWith(), signer.address, {
+                ...TX_PARAMS,
+                gas: "0x30d40",
+                gasLimit: "0x5208",
+            }),
+        ).rejects.toThrow(
+            "The site asked for two different gas limits for this transaction.",
+        );
+    });
+
     test("carries an access list the request asked for", async () => {
         const approved = await prepareApprovalTx(
             providerWith(),
@@ -212,6 +249,15 @@ describe("prepareApprovalTx", () => {
         ).rejects.toThrow(/gas limit no network this wallet supports/);
     });
 
+    test("refuses a gas limit the site sent above the ceiling", async () => {
+        await expect(
+            prepareApprovalTx(providerWith(), signer.address, {
+                ...TX_PARAMS,
+                gas: toQuantity(MAX_GAS_LIMIT + 1n),
+            }),
+        ).rejects.toThrow(/gas limit no network this wallet supports/);
+    });
+
     // The combined bound at population: a gas limit and a fee that are each
     // under their own ceiling but multiply to thousands of ETH is refused
     // before the approval window opens, so the user is never shown a

tests/approvalVerify.test.js

@@ -58,7 +58,6 @@ const TX_PARAMS = {
     to: RECIPIENT,
     value: "0x2386f26fc10000",
     data: "0xdeadbeef",
-    gas: "0x5208",
 };
 
 // The values populateTransaction() fills in when the dApp fixed none of them.

tests/backgroundApproval.test.js

@@ -397,7 +397,13 @@ function loadBackground(options) {
                 rpcResult = r;
             },
         );
-        return { result: () => rpcResult };
+        return {
+            id: () =>
+                new URL(created[created.length - 1].url).searchParams.get(
+                    "approval",
+                ),
+            result: () => rpcResult,
+        };
     }
 
     // A dApp asking to connect. The origin defaults to one the persisted
@@ -1484,6 +1490,55 @@ describe("what the approval is verified against", () => {
         });
     });
 
+    // Typed data whose domain names another chain is refused on the sign
+    // screen; a signature for it that reaches the background anyway is not
+    // handed to the page (https://git.eeqj.de/sneak/AutistMask/issues/519).
+    // TYPED_DATA names mainnet.
+    async function answerTypedData(bg, pending) {
+        const { domain, types, message } = JSON.parse(TYPED_DATA);
+        delete types.EIP712Domain;
+        const signature = await signer.signTypedData(domain, types, message);
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id: pending.id(),
+                approved: true,
+                signature,
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        return { answer, signature };
+    }
+
+    test("typed data for the active chain is handed back signed", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTypedData();
+        await settle();
+
+        const { answer, signature } = await answerTypedData(bg, pending);
+
+        expect(answer.sendResponse).toHaveBeenCalledWith({ signature });
+        expect(pending.result()).toEqual({ result: signature });
+    });
+
+    test("typed data for another chain is refused, not handed back", async () => {
+        const bg = loadBackground();
+        bg.setNetwork(SEPOLIA);
+        const pending = bg.requestTypedData();
+        await settle();
+
+        const { answer } = await answerTypedData(bg, pending);
+
+        const refusal =
+            "This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: refusal,
+            retryable: false,
+        });
+        expect(pending.result()).toEqual({ error: { message: refusal } });
+    });
+
     test("a sign request from another address raises no approval at all", async () => {
         const bg = loadBackground();
         const pending = bg.requestSign(other.address);
@@ -1551,6 +1606,55 @@ describe("what the approval is verified against", () => {
     });
 });
 
+// eth_sign names the address first and the data second, the reverse of
+// personal_sign, and is answered as the JSON-RPC specification defines it:
+// with an EIP-191 personal message signature over the data, the same
+// signature personal_sign returns
+// (https://git.eeqj.de/sneak/AutistMask/issues/409). The signature below is
+// made by the test; that the sign screen makes the same one is tested in
+// tests/ethSign.test.js.
+describe("eth_sign", () => {
+    test("is read address first, and returns a personal message signature over the data", async () => {
+        const bg = loadBackground();
+        const page = bg.send(
+            {
+                type: "AUTISTMASK_RPC",
+                method: "eth_sign",
+                params: [signer.address, MESSAGE],
+            },
+            { origin: ORIGIN },
+        );
+        await settle();
+        const id = approvalIdIn(bg.created.map((c) => c.url));
+
+        const details = bg.send(
+            { type: "AUTISTMASK_GET_APPROVAL", id },
+            { url: bg.fromPopup.url },
+        );
+        const { signParams } = details.sendResponse.mock.calls[0][0];
+        expect(signParams).toMatchObject({
+            method: "eth_sign",
+            message: MESSAGE,
+            from: signer.address,
+        });
+        expect(signParams.dangerWarning).toBe(
+            "This site used the older eth_sign method. " +
+                "The data will be signed as a personal message.",
+        );
+
+        const signature = await signer.signMessage(
+            Buffer.from(MESSAGE.slice(2), "hex"),
+        );
+        bg.send(
+            { type: "AUTISTMASK_SIGN_RESPONSE", id, approved: true, signature },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(page.sendResponse).toHaveBeenCalledWith({ result: signature });
+    });
+});
+
 // The interlock must not cost the retry the approval exists to allow.
 describe("the interlock releases a failed attempt", () => {
     test("a retryable failure before the broadcast leaves the approval usable", async () => {

tests/confirmTxNetworkSwitch.test.js

@@ -0,0 +1,259 @@
+// A transfer is sent only on the network its confirmation screen was drawn for
+// (https://git.eeqj.de/sneak/AutistMask/issues/524).
+//
+// Settings is reachable from the confirmation screen, and Back from it only
+// unhides the screen, still labelled for the network it was drawn for. Sign &
+// Send used to take the network at the moment of the click, so 1 SepoliaETH
+// reviewed on Sepolia went out as 1 ETH on mainnet once mainnet was chosen in
+// Settings in between.
+//
+// Driven through the real confirmation screen, signer and chain switch, with
+// only the node and the DOM stubbed. The stub node is on the network its
+// provider was built for, so a broadcast records the network it went to.
+
+"use strict";
+
+// The signed transactions the stub node was handed, with its network's chain id.
+const mockNode = { broadcast: [] };
+
+jest.mock("ethers", () => {
+    const actual = jest.requireActual("ethers");
+    class StubProvider {
+        constructor(url, network) {
+            this.network = network;
+        }
+        async getNetwork() {
+            return this.network;
+        }
+        async getFeeData() {
+            return {
+                maxFeePerGas: 20000000000n,
+                maxPriorityFeePerGas: 1000000000n,
+                gasPrice: 10000000000n,
+            };
+        }
+        async estimateGas() {
+            return 21000n;
+        }
+        async getCode() {
+            return "0x";
+        }
+        async getTransactionCount() {
+            return 1;
+        }
+        async lookupAddress() {
+            return null;
+        }
+        async broadcastTransaction(signed) {
+            mockNode.broadcast.push({ chainId: this.network.chainId, signed });
+            return { hash: "0x" + "ab".repeat(32) };
+        }
+    }
+    return { ...actual, JsonRpcProvider: StubProvider };
+});
+
+jest.mock("../src/shared/log", () => ({
+    log: {
+        debugf: () => {},
+        infof: () => {},
+        warnf: () => {},
+        errorf: () => {},
+    },
+    debugFetch: jest.fn(async () => ({
+        ok: true,
+        status: 200,
+        json: async () => ({ items: [] }),
+    })),
+    urlOrigin: () => "",
+    setRuntimeDebug: () => {},
+    isDebug: () => false,
+}));
+
+// The wait screen polls for a receipt; these tests stop at the broadcast.
+jest.mock("../src/popup/views/txStatus", () => ({
+    showWait: jest.fn(),
+    showError: jest.fn(),
+}));
+
+global.fetch = jest.fn(() => {
+    throw new Error("tests must not perform network requests");
+});
+
+const { makeStorageStub } = require("./support/storageStub");
+global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
+
+// A stub DOM: every id resolves to a recording element.
+const elements = new Map();
+
+function makeEl(id) {
+    const handlers = new Map();
+    return {
+        id,
+        textContent: "",
+        innerHTML: "",
+        value: "",
+        disabled: false,
+        style: {},
+        dataset: {},
+        classList: {
+            add() {},
+            remove() {},
+            toggle() {},
+            contains: () => false,
+        },
+        handlers,
+        addEventListener(name, fn) {
+            handlers.set(name, fn);
+        },
+        querySelectorAll: () => [],
+        querySelector: () => null,
+        remove() {},
+        focus() {},
+    };
+}
+
+global.document = {
+    getElementById(id) {
+        if (!elements.has(id)) elements.set(id, makeEl(id));
+        return elements.get(id);
+    },
+    createElement: (tag) => makeEl(tag),
+    body: { prepend() {}, appendChild() {} },
+    addEventListener() {},
+};
+global.navigator = { clipboard: { writeText() {} } };
+
+const { BaseWallet, Transaction, Wallet } = require("ethers");
+const { encryptWithPassword } = require("../src/shared/vault");
+const { state, loadState } = require("../src/shared/state");
+const { onChainSwitch } = require("../src/shared/chainSwitch");
+const { NETWORKS } = require("../src/shared/networks");
+const txStatus = require("../src/popup/views/txStatus");
+const confirmTx = require("../src/popup/views/confirmTx");
+
+const PRIVATE_KEY = "0x" + "11".repeat(32);
+const HOLDER = new Wallet(PRIVATE_KEY).address;
+const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
+const PASSWORD = "correct horse battery staple";
+
+function el(id) {
+    return global.document.getElementById(id);
+}
+
+let encryptedSecret;
+let signTransaction;
+
+beforeAll(async () => {
+    await loadState();
+    encryptedSecret = await encryptWithPassword(PRIVATE_KEY, PASSWORD);
+});
+
+beforeEach(() => {
+    elements.clear();
+    mockNode.broadcast = [];
+    txStatus.showWait.mockClear();
+    txStatus.showError.mockClear();
+    signTransaction = jest.spyOn(BaseWallet.prototype, "signTransaction");
+});
+
+afterEach(() => {
+    signTransaction.mockRestore();
+});
+
+// Show the confirmation screen for 1 of the native token on `networkId`, as
+// Review on the Send screen does, with its fee estimate settled.
+async function review(networkId) {
+    await onChainSwitch(networkId);
+    state.wallets = [
+        {
+            type: "key",
+            name: "Wallet 1",
+            encryptedSecret,
+            addresses: [{ address: HOLDER, balance: "5.0" }],
+        },
+    ];
+    state.selectedWallet = 0;
+    state.selectedAddress = 0;
+    confirmTx.init({});
+    confirmTx.show({
+        from: HOLDER,
+        to: RECIPIENT,
+        ensName: null,
+        amount: "1",
+        token: "ETH",
+        balance: "5.0",
+        tokenSymbol: null,
+        tokenBalance: null,
+        tokenDecimals: null,
+        max: false,
+    });
+    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
+}
+
+async function signAndSend() {
+    el("confirm-tx-password").value = PASSWORD;
+    await el("btn-confirm-send").handlers.get("click")();
+}
+
+test("a transfer reviewed on Sepolia and sent goes out on Sepolia", async () => {
+    await review("sepolia");
+    expect(el("confirm-type").textContent).toBe("Native SepoliaETH transfer");
+    expect(el("btn-confirm-send").disabled).toBe(false);
+    await signAndSend();
+    expect(mockNode.broadcast).toHaveLength(1);
+    expect(mockNode.broadcast[0].chainId).toBe(11155111n);
+    expect(Transaction.from(mockNode.broadcast[0].signed).chainId).toBe(
+        11155111n,
+    );
+    expect(txStatus.showWait).toHaveBeenCalledTimes(1);
+});
+
+describe.each([
+    ["sepolia", "mainnet", "SepoliaETH"],
+    ["mainnet", "sepolia", "ETH"],
+])(
+    "a transfer reviewed on %s, with %s chosen in Settings before Sign & Send",
+    (reviewedOn, switchedTo, symbol) => {
+        test("is neither signed nor sent, and the screen says why", async () => {
+            await review(reviewedOn);
+            expect(el("confirm-type").textContent).toBe(
+                "Native " + symbol + " transfer",
+            );
+            // Settings' network select, then Back, which only unhides the
+            // screen as it was drawn.
+            await onChainSwitch(switchedTo);
+            await signAndSend();
+
+            expect(signTransaction).not.toHaveBeenCalled();
+            expect(mockNode.broadcast).toEqual([]);
+            expect(txStatus.showWait).not.toHaveBeenCalled();
+            expect(txStatus.showError).not.toHaveBeenCalled();
+            expect(el("confirm-errors").style.visibility).toBe("visible");
+            expect(el("confirm-errors").innerHTML).toContain(
+                "The active network changed after this transaction was" +
+                    " reviewed, so it was not sent. Please go back and review" +
+                    " it again.",
+            );
+        });
+    },
+);
+
+// The check in the click handler tells the user; the send itself carries the
+// chain id the screen was drawn for, so a signer on another network's node
+// cannot sign it even if the network changes after that check.
+test("the send is refused by a signer on another network's node", async () => {
+    const { getProvider } = require("../src/shared/balances");
+    const signer = new Wallet(PRIVATE_KEY).connect(
+        getProvider("https://rpc.example.invalid", "mainnet"),
+    );
+    await expect(
+        confirmTx.populateVerifyAndSend(signer, {
+            token: "ETH",
+            to: RECIPIENT,
+            amount: "1",
+            chainId: NETWORKS.sepolia.chainId,
+        }),
+    ).rejects.toThrow("transaction chainId mismatch");
+    expect(signTransaction).not.toHaveBeenCalled();
+    expect(mockNode.broadcast).toEqual([]);
+});

tests/ethSign.test.js

@@ -0,0 +1,102 @@
+// The sign screen signs an eth_sign request the way it signs personal_sign: as
+// an EIP-191 personal message over the bytes of the data
+// (https://git.eeqj.de/sneak/AutistMask/issues/409). Booted through the real
+// popup entry point as the approval window. The background's half, which reads
+// the address first and the data second and checks the signature, is tested in
+// tests/backgroundApproval.test.js.
+
+// Mocked so that no password has to be hashed: the stored secret decrypts to
+// the test key below.
+jest.mock("../src/shared/vault", () => ({
+    decryptWithPassword: jest.fn(),
+    encryptWithPassword: jest.fn(),
+}));
+
+const { Wallet, getBytes } = require("ethers");
+const {
+    bootPopup,
+    cleanupPopup,
+    unversionedValidProfile,
+} = require("./support/popupBoot");
+
+// A published test key, never used for real funds.
+const SIGNER_KEY =
+    "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
+const signer = new Wallet(SIGNER_KEY);
+// "Hello AutistMask", as the hex a page sends.
+const MESSAGE = "0x48656c6c6f204175746973744d61736b";
+const ORIGIN = "https://dapp.example";
+
+afterEach(cleanupPopup);
+
+test("the sign screen answers eth_sign with a personal message signature over the data", async () => {
+    const sent = [];
+    const page = await bootPopup(
+        unversionedValidProfile({
+            wallets: [
+                {
+                    type: "key",
+                    name: "Wallet 1",
+                    encryptedSecret: "encrypted-key",
+                    addresses: [
+                        {
+                            address: signer.address,
+                            balance: "0.0000",
+                            tokenBalances: [],
+                        },
+                    ],
+                },
+            ],
+            activeAddress: signer.address,
+            allowedSites: { [signer.address]: [ORIGIN] },
+        }),
+        {
+            search: "?approval=approval-1",
+            runtime: {
+                connect: () => ({ postMessage: () => {} }),
+                // Answered as the background answers: with the eth_sign
+                // approval this window was opened for, and by handing back
+                // the signature the popup sends.
+                sendMessage: (msg, reply) => {
+                    sent.push(msg);
+                    if (msg.type === "AUTISTMASK_GET_APPROVAL") {
+                        reply({
+                            type: "sign",
+                            origin: ORIGIN,
+                            isPhishingDomain: false,
+                            approvedFrom: signer.address,
+                            signParams: {
+                                method: "eth_sign",
+                                message: MESSAGE,
+                                from: signer.address,
+                            },
+                        });
+                    } else if (msg.type === "AUTISTMASK_SIGN_RESPONSE") {
+                        reply({ signature: msg.signature });
+                    } else {
+                        reply(undefined);
+                    }
+                },
+            },
+        },
+    );
+    globalThis.window.close = jest.fn();
+    const { decryptWithPassword } = require("../src/shared/vault");
+    decryptWithPassword.mockResolvedValue(SIGNER_KEY);
+
+    expect(page.visibleViews()).toEqual(["approve-sign"]);
+    page.node("approve-sign-password").value = "any password";
+    await page.click("btn-approve-sign");
+
+    expect(
+        sent.filter((msg) => msg.type === "AUTISTMASK_SIGN_RESPONSE"),
+    ).toEqual([
+        {
+            type: "AUTISTMASK_SIGN_RESPONSE",
+            id: "approval-1",
+            approved: true,
+            signature: await signer.signMessage(getBytes(MESSAGE)),
+        },
+    ]);
+    expect(globalThis.window.close).toHaveBeenCalled();
+});

tests/phishingDomains.test.js

@@ -89,6 +89,12 @@ describe("vendored blocklist", () => {
         expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
     });
 
+    test("a hostname ending in a dot is the same host", () => {
+        expect(isPhishingDomain(LISTED[1] + ".")).toBe(true);
+        expect(isPhishingDomain("wallet." + LISTED[1] + ".")).toBe(true);
+        expect(isPhishingDomain(CLEAN[0] + ".")).toBe(false);
+    });
+
     test("returns false for an empty or missing hostname", () => {
         expect(isPhishingDomain("")).toBe(false);
         expect(isPhishingDomain(null)).toBe(false);
@@ -133,6 +139,13 @@ describe("hostnameVariants", () => {
     test("lowercases hostnames", () => {
         expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
     });
+
+    test("drops the trailing dot of a fully qualified hostname", () => {
+        expect(hostnameVariants("sub.evil.com.")).toEqual([
+            "sub.evil.com",
+            "evil.com",
+        ]);
+    });
 });
 
 describe("domain hashing", () => {

tests/tokenTransferDetail.test.js

@@ -0,0 +1,176 @@
+// The decoded details on the transaction detail screen of a token transfer
+// (https://git.eeqj.de/sneak/AutistMask/issues/515).
+//
+// A history row for a token transfer holds the transfer's recipient as its
+// `to`, not the contract the transaction was sent to. The screen used to decode
+// the transaction's data against that `to`, so its Token line named the
+// recipient, and the amount read "decimals unknown" for a token the wallet
+// tracks. It decodes against the address the explorer reports the transaction
+// was sent to.
+//
+// The row is the one the real history parser makes from the explorer's answer,
+// opened through transactionDetail.show(), which is what a click on the row
+// calls. Driven against a minimal DOM stub in the shape
+// tests/contractCreation.test.js uses.
+
+const { Interface } = require("ethers");
+
+jest.mock("../src/shared/log", () => ({
+    log: {
+        debugf: () => {},
+        infof: () => {},
+        warnf: () => {},
+        errorf: () => {},
+    },
+    debugFetch: async (url) => ({
+        ok: true,
+        json: async () => mockExplorerAnswer(url),
+    }),
+    setRuntimeDebug: () => {},
+    isDebug: () => false,
+}));
+
+globalThis.chrome = {
+    storage: { local: { get: async () => ({}), set: async () => {} } },
+};
+
+const { state } = require("../src/shared/state");
+const { fetchRecentTransactions } = require("../src/shared/transactions");
+const transactionDetail = require("../src/popup/views/transactionDetail");
+
+const EXPLORER = "https://explorer.example/api/v2";
+const HOLDER = "0x0000000000000000000000000000000000000a11";
+const RECIPIENT = "0x711e691962E5c054b78071C789A0C8CE657377aD";
+// A token the wallet tracks and the bundled token list does not name.
+const TOKEN = "0x74de5d4FCbf63E00296fd95d33236B9794016631";
+const TX_HASH =
+    "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84";
+
+// HOLDER sends 1234.5 of the 6-decimal token to RECIPIENT.
+const TRANSFER_DATA = new Interface([
+    "function transfer(address to, uint256 amount)",
+]).encodeFunctionData("transfer", [RECIPIENT, 1234500000n]);
+
+// The explorer's answers: the transfer in HOLDER's token transfers, and the
+// transaction itself, sent to the token contract.
+function mockExplorerAnswer(url) {
+    if (
+        url ===
+        EXPLORER + "/addresses/" + HOLDER + "/token-transfers?type=ERC-20"
+    ) {
+        return {
+            items: [
+                {
+                    transaction_hash: TX_HASH,
+                    block_number: 100,
+                    timestamp: "2026-10-01T00:00:00Z",
+                    from: { hash: HOLDER },
+                    to: { hash: RECIPIENT },
+                    total: { value: "1234500000", decimals: "6" },
+                    token: {
+                        address_hash: TOKEN,
+                        symbol: "TDOL",
+                        holders_count: "5000",
+                    },
+                },
+            ],
+        };
+    }
+    if (url === EXPLORER + "/transactions/" + TX_HASH) {
+        return { to: { hash: TOKEN }, raw_input: TRANSFER_DATA };
+    }
+    return { items: [] };
+}
+
+function makeElement(id) {
+    const classes = new Set();
+    const el = {
+        id,
+        textContent: "",
+        value: "",
+        innerHTML: "",
+        style: {},
+        dataset: {},
+        classList: {
+            add: (...names) => names.forEach((n) => classes.add(n)),
+            remove: (...names) => names.forEach((n) => classes.delete(n)),
+            contains: (n) => classes.has(n),
+            toggle: () => false,
+        },
+        addEventListener: () => {},
+        querySelectorAll: () => [],
+        appendChild: () => {},
+    };
+    // Views reach for .parentElement to hide whole sections.
+    Object.defineProperty(el, "parentElement", {
+        get: () => node(id + "-parent"),
+    });
+    return el;
+}
+
+function makeDocument() {
+    const els = new Map();
+    return {
+        getElementById(id) {
+            // The debug banner is created on demand by helpers.js; absent
+            // is the state a non-debug, non-testnet popup is in.
+            if (id === "debug-banner") return null;
+            if (!els.has(id)) els.set(id, makeElement(id));
+            return els.get(id);
+        },
+        createElement: () => makeElement("created"),
+        body: { prepend: () => {} },
+    };
+}
+
+function node(id) {
+    return globalThis.document.getElementById(id);
+}
+
+// One line of the decoded details: its label and what is drawn under it.
+function decodedLine(label) {
+    return node("tx-detail-calldata-details")
+        .innerHTML.split('<div class="mb-2">')
+        .find((line) =>
+            line.startsWith(`<div class="text-muted">${label}</div>`),
+        );
+}
+
+beforeEach(() => {
+    globalThis.document = makeDocument();
+    globalThis.window = { location: { search: "" } };
+    state.blockscoutUrl = EXPLORER;
+    state.wallets = [
+        {
+            name: "Main",
+            type: "key",
+            addresses: [{ address: HOLDER, balance: "0.0000" }],
+        },
+    ];
+    state.trackedTokens = [{ address: TOKEN, symbol: "TDOL", decimals: 6 }];
+    state.viewData = {};
+    state.viewStack = [];
+    state.currentView = null;
+});
+
+test("a token transfer's decoded details name the token contract and the amount at its scale", async () => {
+    const [row] = await fetchRecentTransactions(HOLDER, EXPLORER, 1);
+    expect(row.to).toBe(RECIPIENT);
+    expect(row.contractAddress).toBe(TOKEN.toLowerCase());
+
+    transactionDetail.show(row);
+    // The decoded details are drawn once the explorer has answered.
+    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
+
+    expect(node("tx-detail-calldata-action").textContent).toBe(
+        "Token Transfer",
+    );
+    const token = decodedLine("Token");
+    expect(token).toContain('<div class="font-bold">TDOL</div>');
+    expect(token).toContain(TOKEN);
+    expect(token).not.toContain(RECIPIENT);
+    expect(decodedLine("Recipient")).toContain(RECIPIENT);
+    expect(decodedLine("Amount")).toContain(
+        '<div class="font-bold">1234.5000 TDOL</div>',
+    );
+});

tests/typedDataPermit.test.js

@@ -233,6 +233,26 @@ function warningOf(data) {
     return html.slice(0, html.indexOf(">Domain<"));
 }
 
+const NOT_SIGNED = ">Not part of the signature<";
+
+// The Message section alone, without the keys listed after it as not part of
+// the signature.
+function messageOf(data) {
+    const html = formatTypedDataHtml(JSON.stringify(data));
+    const end = html.indexOf(NOT_SIGNED);
+    return html.slice(
+        html.indexOf(">Message<"),
+        end === -1 ? html.length : end,
+    );
+}
+
+// The keys listed as not part of the signature, or "" when there are none.
+function notSignedOf(data) {
+    const html = formatTypedDataHtml(JSON.stringify(data));
+    const start = html.indexOf(NOT_SIGNED);
+    return start === -1 ? "" : html.slice(start);
+}
+
 function request(data) {
     return { method: "eth_signTypedData_v4", typedData: JSON.stringify(data) };
 }
@@ -240,6 +260,7 @@ function request(data) {
 beforeEach(() => {
     state.trackedTokens = [];
     state.wallets = [];
+    state.networkId = "mainnet";
 });
 
 describe("a token permission is warned about, naming spender and amount", () => {
@@ -311,7 +332,8 @@ describe("the warning reads only the fields the signed type declares", () => {
     });
 
     // A Permit whose type names its spender `operator`; the page adds a
-    // `spender` key the type does not declare.
+    // `spender` key the type does not declare. The Message section names
+    // only the operator, and the added key is listed apart as not signed.
     test("an added key is not named as the spender", () => {
         const data = {
             ...PERMIT,
@@ -330,6 +352,10 @@ describe("the warning reads only the fields the signed type declares", () => {
         const warning = warningOf(data);
         expect(warning).toContain(WARNING);
         expect(warning).not.toContain(DECOY);
+        expect(messageOf(data)).toContain(SPENDER);
+        expect(messageOf(data)).not.toContain(DECOY);
+        expect(notSignedOf(data)).toContain("spender:");
+        expect(notSignedOf(data)).toContain(DECOY);
     });
 
     // The permit for a Uniswap v3 position NFT: a `Permit` with no amount
@@ -370,6 +396,47 @@ describe("the warning reads only the fields the signed type declares", () => {
     });
 });
 
+// ethers signs only the declared fields at every level of nesting, so an
+// added key inside a struct is not signed either. Such typed data is not
+// refused; the added key is listed apart, with where it sits in the message.
+describe("the Message section shows only the fields that are signed", () => {
+    const NOTE = "This signature only signs you in";
+
+    test("an added key inside a nested struct is listed as not signed", () => {
+        const data = {
+            ...PERMIT_SINGLE,
+            message: {
+                ...PERMIT_SINGLE.message,
+                details: { ...PERMIT_SINGLE.message.details, note: NOTE },
+            },
+        };
+        expect(messageOf(data)).toContain("expiration");
+        expect(messageOf(data)).not.toContain(NOTE);
+        expect(notSignedOf(data)).toContain("details.note:");
+        expect(notSignedOf(data)).toContain(NOTE);
+        expect(typedDataRefusal(request(data))).toBeNull();
+    });
+
+    test("an added key inside a list item is listed with its index", () => {
+        const [first, second] = PERMIT_BATCH.message.details;
+        const data = {
+            ...PERMIT_BATCH,
+            message: {
+                ...PERMIT_BATCH.message,
+                details: [first, { ...second, note: NOTE }],
+            },
+        };
+        expect(messageOf(data)).not.toContain(NOTE);
+        expect(notSignedOf(data)).toContain("details[1].note:");
+        expect(notSignedOf(data)).toContain(NOTE);
+    });
+
+    test("nothing is listed apart when every key is declared", () => {
+        expect(notSignedOf(PERMIT_SINGLE)).toBe("");
+        expect(notSignedOf(LOGIN)).toBe("");
+    });
+});
+
 describe("the primary type shown is the one ethers signs", () => {
     // A drain stated as a sign-in: the page says Login, the types say
     // PermitSingle, and ethers would sign PermitSingle.
@@ -532,3 +599,66 @@ describe("the sign screen refuses a mismatched primary type", () => {
         expect(node("btn-approve-sign").disabled).toBe(true);
     });
 });
+
+// EIP-712: a wallet refuses typed data whose domain names a chain other than
+// the active one, because the signature is valid on the chain it names
+// (https://git.eeqj.de/sneak/AutistMask/issues/519). The wallet is on Sepolia
+// and the site asks for a mainnet USDC Permit.
+describe("the sign screen refuses typed data for another chain", () => {
+    const REFUSAL =
+        "This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
+
+    function withChainId(chainId) {
+        return { ...PERMIT, domain: { ...PERMIT.domain, chainId } };
+    }
+
+    beforeEach(() => {
+        decryptWithPassword.mockClear();
+        state.networkId = "sepolia";
+    });
+
+    test("another chain shows the error, with Sign disabled", async () => {
+        await openSignScreen(PERMIT);
+        expect(node("approve-sign-error").textContent).toBe(REFUSAL);
+        expect(node("btn-approve-sign").disabled).toBe(true);
+    });
+
+    test("Sign clicked anyway decrypts and signs nothing", async () => {
+        const sent = await openSignScreen(PERMIT);
+        node("approve-sign-password").value = "any password";
+        await click("btn-approve-sign");
+
+        expect(decryptWithPassword).not.toHaveBeenCalled();
+        expect(sent.map((msg) => msg.type)).not.toContain(
+            "AUTISTMASK_SIGN_RESPONSE",
+        );
+        expect(node("approve-sign-error").textContent).toBe(REFUSAL);
+    });
+
+    // ethers signs 1, "1" and "0x1" as the same chain id.
+    test("the chain id is refused in each form ethers signs", () => {
+        for (const chainId of [1, "1", "0x1"]) {
+            expect(typedDataRefusal(request(withChainId(chainId)))).toBe(
+                REFUSAL,
+            );
+        }
+    });
+
+    test("a chain id that is not a number is refused", () => {
+        expect(typedDataRefusal(request(withChainId("mainnet")))).toBe(
+            "This typed data cannot be read, so it cannot be signed.",
+        );
+    });
+
+    test("the active chain leaves Sign enabled", async () => {
+        await openSignScreen(withChainId("0xaa36a7"));
+        expect(node("approve-sign-error").textContent).toBe("");
+        expect(node("btn-approve-sign").disabled).toBe(false);
+    });
+
+    test("no chain id leaves Sign enabled", async () => {
+        await openSignScreen(withChainId(undefined));
+        expect(node("approve-sign-error").textContent).toBe("");
+        expect(node("btn-approve-sign").disabled).toBe(false);
+    });
+});

tests/walletDefects.test.js

@@ -153,6 +153,18 @@ describe("the explanatory copy", () => {
         expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
     });
 
+    // The wallet holds the imported key but will not export it, so the copy
+    // must name every key the user may hold that reaches the addresses
+    // outside it.
+    test("it names the keys that reach the addresses outside this wallet", () => {
+        const body = defect.paragraphs.join(" ");
+
+        expect(body).toMatch(/private key export are disabled/);
+        expect(body).toMatch(
+            /with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
+        );
+    });
+
     test("it says the wallet is not deleted and what the user can do", () => {
         const body = defect.paragraphs.join(" ");