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

.gitea/workflows/e2e.yml

@@ -35,10 +35,10 @@ on: [push]
 jobs:
     e2e-chrome:
         runs-on: ubuntu-latest
-        # Bounds the image build, a cold cache included, and the three Chrome
+        # Bounds the image build, a cold cache included, and both Chrome
         # programs, so a hung browser frees the shared runner. README.md
         # "In CI" has the measured times.
-        timeout-minutes: 40
+        timeout-minutes: 20
         steps:
             # actions/checkout v4.2.2, 2026-02-22
             - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683

README.md

@@ -347,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
@@ -527,24 +535,6 @@ one profile yields a single extension id, not two: Chrome does not load a second
 copy of an id it already has. The assertions are annotated as observations, so a
 Chrome that ever changes this fails the run instead of passing it.
 
-Last, `make test-e2e` runs `tests/e2e/slowApproval.js`, which leaves a
-transaction approval and a signature approval open for seven minutes, then
-approves both and requires the site to receive the hash of the transaction the
-stub node received and a signature that recovers to the address. Before the
-approval window kept the background worker running, Chrome stopped it about six
-minutes in and neither request was ever answered
-([#540](https://git.eeqj.de/sneak/AutistMask/issues/540)). It is its own program
-and does not use Playwright: Playwright keeps DevTools attached to the
-background worker, and Chrome never stops a worker DevTools is attached to, so
-no wait in the suite above can show the worker stopping. It starts Chromium
-itself, speaks the DevTools protocol over `--remote-debugging-pipe`, and
-attaches to pages only. Before raising the approvals it requires the worker to
-stop on its own with nothing pending, so a run in which something else keeps the
-worker running fails. The site is connected and the RPC endpoint set through the
-extension's own screens, against the Firefox suite's loopback dApp page and stub
-node (`tests/e2e/firefox/dapp.js`). Unlike the suite above, it does not fail on
-page errors.
-
 ### Firefox (`make test-e2e-firefox`)
 
 `make test-e2e-firefox` builds `dist/firefox/` and drives the **real popup in a
@@ -725,13 +715,11 @@ installing Firefox, geckodriver and their system libraries. Both scripts now
 build their image with `--no-cache`, so every run pays the cold figure. Those
 `e2e-chrome` runs predate the cases that wait in real time for a receipt to end
 in error. `make test-e2e` took 3m51s locally with its image cached, so with the
-image rebuilt every run, an `e2e-chrome` run comes to about seven minutes. The
-approvals left open for seven minutes (`tests/e2e/slowApproval.js`) add about
-eight more, so about fifteen.
+image rebuilt every run, an `e2e-chrome` run comes to about seven minutes.
 
 Each e2e job has a `timeout-minutes` cap, so a hung build or browser ends the
 job instead of holding the shared runner: `e2e-firefox` 15 minutes and
-`e2e-chrome` 40, each over two and a half times the job's slowest cold run. A
+`e2e-chrome` 20, each over two and a half times the job's slowest cold run. A
 job that reaches its cap has hung; read it as a hang, not as a slow run to
 retry. The `check` job has no cap: `.gitea/workflows/check.yml` is the canonical
 copy from `sneak/prompts`, kept byte-identical.
@@ -826,16 +814,6 @@ on the next event. Two consequences shape every recurring job in the background:
   service worker at all — the one remaining user of it, `src/shared/ens.js`,
   runs only in the popup and is marked as such.
 
-Pending approvals are the exception: an approval waiting for the user, and the
-site's request waiting on it, live only in the worker's memory, and only the
-worker that received the request can answer it. Chrome stopped the worker about
-six minutes after a site's request even with the approval window still open, and
-the user's answer then reached nobody. So while an approval window is open it
-posts an empty message on its approval port every 20 seconds, and each message
-is an event that keeps the worker running: an approval stays answerable however
-long the user takes. A worker that stops for another reason, a crash say, still
-loses the approvals open in it.
-
 The job also carries a freshness guard, and a guard must never be timed to the
 alarm period it gates. The guard is measured from the moment the last run
 finished, which is one run-duration after the alarm that started it, so a guard
@@ -1675,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**
@@ -2105,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
@@ -2158,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
@@ -2173,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

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,22 +104,11 @@ 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
-
-    # Approvals left open for seven minutes, past the point where Chrome
-    # stopped the background worker before the approval window kept it
-    # running. Its own program, without Playwright: Playwright keeps DevTools
-    # attached to the worker, and Chrome never stops a worker DevTools is
-    # attached to. It reaches nothing but its own loopback server.
-    echo "Running the approvals left open for seven minutes..."
-    docker run --rm \
-        --ipc=host \
-        -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
-        node tests/e2e/slowApproval.js
 }
 
 main "$@"

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

@@ -649,9 +649,6 @@ runtime.onConnect.addListener((port) => {
             pendingApprovals[id].portConnected = true;
         }
         port.onMessage.addListener((msg) => {
-            // The window also posts an empty message every 20 seconds while
-            // it is open, only so that Chrome keeps this worker, and the
-            // approval in it, running (src/popup/views/approval.js, show()).
             if (!msg || msg.type !== "AUTISTMASK_APPROVAL_DECISION") return;
             if (!isExtensionSender(port.sender)) return;
             const approval = pendingApprovals[id];
@@ -991,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")) {

src/popup/views/approval.js

@@ -626,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;
@@ -663,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;
@@ -725,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) {
@@ -784,22 +830,6 @@ async function show(id) {
     approvalId = id;
     approvalPort = runtimeApi().connect({ name: "approval:" + id });
 
-    // The approval this window answers, and the site's request waiting on it,
-    // exist only in the background worker's memory. Chrome stops that worker
-    // about six minutes after the site's request even while the user is still
-    // deciding, and the approval is lost with it. A message on this port is an
-    // event, and an event keeps the worker running, so one is sent every 20
-    // seconds for as long as this window is open. The background ignores it.
-    const port = approvalPort;
-    const timer = setInterval(() => {
-        try {
-            port.postMessage({});
-        } catch {
-            // The worker is gone anyway, and the port with it.
-            clearInterval(timer);
-        }
-    }, 20000);
-
     let details = null;
     try {
         details = await sendMessage({ type: "AUTISTMASK_GET_APPROVAL", id });

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))

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

@@ -1606,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/e2e/slowApproval.js

@@ -1,495 +0,0 @@
-// A transaction approval and a signature approval the user leaves open for
-// seven minutes are still answered, and the site receives both results
-// (https://git.eeqj.de/sneak/AutistMask/issues/540).
-//
-// Both approvals, and the site's requests waiting on them, live only in the
-// background worker's memory. With nothing else going on, Chrome stopped that
-// worker about six minutes after the site's request, while the window was
-// still open, and the user's answer then reached nobody. The approval window
-// now posts on its approval port every 20 seconds, which keeps the worker
-// running (src/popup/views/approval.js, show()).
-//
-// A separate program from run.js, and not driven by Playwright: Playwright
-// keeps DevTools attached to the background worker, and Chrome never stops a
-// worker DevTools is attached to, so in run.js the worker outlives any wait
-// and a wait there proves nothing. This program starts Chromium itself and
-// speaks the DevTools protocol over the pipe Chromium opens for it
-// (--remote-debugging-pipe), attaching to pages only, never to the worker.
-// Before the approvals are raised it watches the worker stop on its own, so a
-// run in which something else keeps the worker running cannot pass.
-//
-// The dApp page and the stub node are the Firefox suite's loopback ones
-// (tests/e2e/firefox/dapp.js). The site is connected and the node chosen
-// through the extension's own screens, so nothing is written to its storage
-// from outside.
-//
-// Run through script/test-e2e, which builds the extension and the pinned
-// container.
-//
-//   node tests/e2e/slowApproval.js
-
-"use strict";
-
-const fs = require("fs");
-const os = require("os");
-const path = require("path");
-const { spawn } = require("child_process");
-
-const { chromium } = require("playwright-core");
-const {
-    Transaction,
-    getAddress,
-    getBytes,
-    hexlify,
-    parseEther,
-    toQuantity,
-    toUtf8Bytes,
-    verifyMessage,
-} = require("ethers");
-
-const { PASSWORD } = require("./harness");
-const { STUB_COUNTERPARTY } = require("./network");
-const { startDappServer } = require("./firefox/dapp");
-
-const DIST_CHROME = path.resolve(__dirname, "..", "..", "dist", "chrome");
-
-// Chrome stopped the worker six minutes after the site's request when nothing
-// kept it running, so the approvals are left open for longer than that.
-const APPROVAL_WAIT_MS = 7 * 60 * 1000;
-
-// With nothing pending, Chrome stops the worker 30 seconds after its last
-// event; this is how long the program waits to see that.
-const IDLE_STOP_TIMEOUT_MS = 90000;
-
-const SIGN_HEX = hexlify(toUtf8Bytes("AutistMask e2e: a slow signature"));
-
-const checks = [];
-let failed = 0;
-
-function check(name, cond, detail) {
-    checks.push(name);
-    if (cond) {
-        console.log("ok " + checks.length + " - " + name);
-    } else {
-        failed += 1;
-        console.log("not ok " + checks.length + " - " + name);
-        if (detail) console.log("  " + detail);
-    }
-}
-
-function sleep(ms) {
-    return new Promise((resolve) => setTimeout(resolve, ms));
-}
-
-// Poll `probe` until it returns something truthy, and return that.
-async function until(what, probe, timeout = 30000) {
-    const deadline = Date.now() + timeout;
-    for (;;) {
-        const value = await probe().catch(() => null);
-        if (value) return value;
-        if (Date.now() > deadline) {
-            throw new Error(
-                "gave up after " + timeout + "ms waiting for " + what,
-            );
-        }
-        await sleep(250);
-    }
-}
-
-// The DevTools protocol over Chromium's pipe: commands go to its fd 3, and
-// replies and events come back on its fd 4, each one JSON message ended by a
-// NUL byte. Events are not used and are dropped. Returns send(method, params,
-// sessionId), which resolves with the command's result.
-function devtools(browser) {
-    const waiting = new Map();
-    let nextId = 1;
-    let pending = "";
-    const replies = browser.stdio[4];
-    replies.setEncoding("utf8");
-    replies.on("data", (chunk) => {
-        pending += chunk;
-        let end;
-        while ((end = pending.indexOf("\0")) !== -1) {
-            const message = JSON.parse(pending.slice(0, end));
-            pending = pending.slice(end + 1);
-            const caller = waiting.get(message.id);
-            if (!caller) continue;
-            waiting.delete(message.id);
-            if (message.error) {
-                caller.reject(
-                    new Error(caller.method + ": " + message.error.message),
-                );
-            } else {
-                caller.resolve(message.result);
-            }
-        }
-    });
-    replies.on("close", () => {
-        for (const caller of waiting.values()) {
-            caller.reject(new Error("the browser exited"));
-        }
-        waiting.clear();
-    });
-    return (method, params = {}, sessionId) =>
-        new Promise((resolve, reject) => {
-            const id = nextId++;
-            waiting.set(id, { method, resolve, reject });
-            browser.stdio[3].write(
-                JSON.stringify({ id, method, params, sessionId }) + "\0",
-            );
-        });
-}
-
-// Attach to a page and return a function that evaluates an expression in it,
-// awaiting a promise it returns, and resolves with the value.
-async function attach(send, targetId) {
-    const { sessionId } = await send("Target.attachToTarget", {
-        targetId,
-        flatten: true,
-    });
-    return async (expression) => {
-        const r = await send(
-            "Runtime.evaluate",
-            { expression, awaitPromise: true, returnByValue: true },
-            sessionId,
-        );
-        if (r.exceptionDetails) {
-            throw new Error(
-                "evaluating " +
-                    expression.slice(0, 80) +
-                    " threw: " +
-                    JSON.stringify(r.exceptionDetails.exception || {}),
-            );
-        }
-        return r.result.value;
-    };
-}
-
-// Whether the page has loaded and shows the element with this id: the
-// `hidden` class showView() keeps on every other view is on neither it nor
-// anything around it.
-function shows(id) {
-    return (
-        "document.readyState === 'complete' &&" +
-        " !!document.getElementById(" +
-        JSON.stringify(id) +
-        ") && !document.getElementById(" +
-        JSON.stringify(id) +
-        ").closest('.hidden')"
-    );
-}
-
-// Set an input's value and fire the input event typing would fire.
-function typeInto(id, value) {
-    return (
-        "(() => { const el = document.getElementById(" +
-        JSON.stringify(id) +
-        "); el.value = " +
-        JSON.stringify(value) +
-        "; el.dispatchEvent(new Event('input', { bubbles: true })); })()"
-    );
-}
-
-function click(id) {
-    return "document.getElementById(" + JSON.stringify(id) + ").click()";
-}
-
-// The outcome of a request the dApp page started with __dapp.start(), or
-// {settled: "pending"} if it has not settled within `timeout`.
-function settled(key, timeout = 30000) {
-    return (
-        "Promise.race([window.__dapp.settle(" +
-        JSON.stringify(key) +
-        "), new Promise((r) => setTimeout(() => r({ settled: 'pending' }), " +
-        timeout +
-        "))])"
-    );
-}
-
-async function main() {
-    if (!fs.existsSync(path.join(DIST_CHROME, "manifest.json"))) {
-        console.error(
-            "slowApproval: no unpacked build at " +
-                DIST_CHROME +
-                " — run make build first",
-        );
-        process.exitCode = 1;
-        return;
-    }
-
-    const node = await startDappServer();
-    const profile = fs.mkdtempSync(
-        path.join(os.tmpdir(), "autistmask-slow-approval-"),
-    );
-    const browser = spawn(
-        chromium.executablePath(),
-        [
-            "--headless",
-            // The container runs unprivileged, as in harness.js.
-            "--no-sandbox",
-            "--remote-debugging-pipe",
-            "--user-data-dir=" + profile,
-            "--disable-extensions-except=" + DIST_CHROME,
-            "--load-extension=" + DIST_CHROME,
-            // Nothing but the loopback dApp page and stub node can be reached.
-            "--host-resolver-rules=MAP * ~NOTFOUND, EXCLUDE 127.0.0.1",
-            "about:blank",
-        ],
-        { stdio: ["ignore", "ignore", "ignore", "pipe", "pipe"] },
-    );
-
-    try {
-        const send = devtools(browser);
-        const targets = async () =>
-            (await send("Target.getTargets")).targetInfos;
-
-        const worker = await until("the background worker", async () =>
-            (await targets()).find(
-                (t) =>
-                    t.type === "service_worker" &&
-                    t.url.startsWith("chrome-extension://"),
-            ),
-        );
-        // The extension id is the host of the worker's URL, as in harness.js.
-        const popupUrl =
-            "chrome-extension://" +
-            new URL(worker.url).host +
-            "/src/popup/index.html";
-        const workerRunning = async () =>
-            (await targets()).some((t) => t.url === worker.url);
-        const openPage = async (url) => {
-            const { targetId } = await send("Target.createTarget", { url });
-            return { targetId, evaluate: await attach(send, targetId) };
-        };
-        const approvalPages = async () =>
-            (await targets()).filter(
-                (t) => t.type === "page" && t.url.includes("?approval="),
-            );
-
-        // A wallet, through the popup's own screens, and the stub node as its
-        // RPC endpoint, through Settings.
-        const popup = await openPage(popupUrl);
-        await until("the welcome screen", () =>
-            popup.evaluate(shows("view-welcome")),
-        );
-        await popup.evaluate(click("btn-welcome-add"));
-        await until("the add wallet screen", () =>
-            popup.evaluate(shows("view-add-wallet")),
-        );
-        await popup.evaluate(click("btn-generate-phrase"));
-        await until("a generated recovery phrase", () =>
-            popup.evaluate(
-                "document.getElementById('wallet-mnemonic').value.trim()" +
-                    ".split(/\\s+/).length >= 12",
-            ),
-        );
-        await popup.evaluate(typeInto("add-wallet-password", PASSWORD));
-        await popup.evaluate(typeInto("add-wallet-password-confirm", PASSWORD));
-        await popup.evaluate(click("btn-add-wallet-confirm"));
-        await until(
-            "the main screen",
-            () => popup.evaluate(shows("view-main")),
-            60000,
-        );
-        await popup.evaluate(click("btn-settings"));
-        await until("the settings screen", () =>
-            popup.evaluate(shows("view-settings")),
-        );
-        await popup.evaluate(typeInto("settings-rpc", node.rpcUrl));
-        await popup.evaluate(click("btn-save-rpc"));
-        await until("the stub node saved as the RPC endpoint", () =>
-            popup.evaluate(
-                "new Promise((resolve) => chrome.storage.local.get(" +
-                    "'autistmask', (r) => resolve(r.autistmask.rpcUrl === " +
-                    JSON.stringify(node.rpcUrl) +
-                    ")))",
-            ),
-        );
-        await send("Target.closeTarget", { targetId: popup.targetId });
-
-        // The site, connected through the real prompt with "Remember", so
-        // the connection is stored rather than held by the worker.
-        const dapp = await openPage(node.url);
-        await until("the injected provider", () =>
-            dapp.evaluate("!!window.ethereum && !!window.__dapp"),
-        );
-        await dapp.evaluate(
-            "window.__dapp.start('accounts', 'eth_requestAccounts', [])",
-        );
-        const prompt = await until(
-            "the connection prompt",
-            async () => (await approvalPages())[0],
-        );
-        const inPrompt = await attach(send, prompt.targetId);
-        await until("the connection prompt's screen", () =>
-            inPrompt(shows("view-approve-site")),
-        );
-        await inPrompt(
-            "document.getElementById('approve-remember').checked = true",
-        );
-        // The prompt closes itself on the click.
-        await inPrompt(click("btn-approve")).catch(() => {});
-        const accounts = await dapp.evaluate(settled("accounts"));
-        if (accounts.settled !== "resolved") {
-            throw new Error(
-                "the site could not be connected: " + JSON.stringify(accounts),
-            );
-        }
-        const address = getAddress(accounts.result[0]);
-        console.log("# address " + address + ", site " + node.origin);
-
-        // The control: with nothing pending, the worker stops on its own.
-        // Were something else keeping it running, the wait below would
-        // prove nothing.
-        const idleStart = Date.now();
-        const stoppedWhenIdle = await until(
-            "the idle worker to stop",
-            async () => !(await workerRunning()),
-            IDLE_STOP_TIMEOUT_MS,
-        ).catch(() => false);
-        check(
-            "with nothing pending, Chrome stops the background worker on its own",
-            stoppedWhenIdle,
-            "the worker was still running " +
-                IDLE_STOP_TIMEOUT_MS / 1000 +
-                "s after its last event, so something other than the " +
-                "approval windows is keeping it running",
-        );
-        console.log(
-            "# idle worker stopped after " +
-                Math.round((Date.now() - idleStart) / 1000) +
-                "s",
-        );
-
-        // A transaction approval and a signature approval, both open at once.
-        await dapp.evaluate(
-            "window.__dapp.start('tx', 'eth_sendTransaction', [" +
-                JSON.stringify({
-                    from: address,
-                    to: STUB_COUNTERPARTY,
-                    value: toQuantity(parseEther("0.01")),
-                }) +
-                "])",
-        );
-        await until("the transaction approval window", async () =>
-            (await approvalPages()).length === 1 ? true : null,
-        );
-        await dapp.evaluate(
-            "window.__dapp.start('sign', 'personal_sign', [" +
-                JSON.stringify(SIGN_HEX) +
-                ", " +
-                JSON.stringify(address) +
-                "])",
-        );
-        const windows = await until("both approval windows", async () => {
-            const pages = await approvalPages();
-            return pages.length === 2 ? pages : null;
-        });
-        let txWindow = null;
-        let signWindow = null;
-        for (const page of windows) {
-            const evaluate = await attach(send, page.targetId);
-            const view = await until("an approval screen", () =>
-                evaluate(
-                    "(" +
-                        shows("view-approve-tx") +
-                        ") ? 'tx' : (" +
-                        shows("view-approve-sign") +
-                        ") ? 'sign' : null",
-                ),
-            );
-            if (view === "tx") txWindow = evaluate;
-            if (view === "sign") signWindow = evaluate;
-        }
-        if (!txWindow || !signWindow) {
-            throw new Error("the two windows are not one of each kind");
-        }
-
-        // The user takes seven minutes. The worker is checked every two
-        // seconds, and the wait ends early if Chrome stops it.
-        const opened = Date.now();
-        let stoppedAfter = null;
-        while (Date.now() - opened < APPROVAL_WAIT_MS) {
-            if (!(await workerRunning())) {
-                stoppedAfter = Math.round((Date.now() - opened) / 1000);
-                break;
-            }
-            await sleep(2000);
-        }
-        check(
-            "the background worker keeps running while both approvals are open",
-            stoppedAfter === null,
-            "Chrome stopped the worker " +
-                stoppedAfter +
-                "s after the approvals opened",
-        );
-
-        await txWindow(typeInto("approve-tx-password", PASSWORD));
-        await txWindow(click("btn-approve-tx"));
-        await signWindow(typeInto("approve-sign-password", PASSWORD));
-        // The window closes itself once the signature is handed over.
-        await signWindow(click("btn-approve-sign")).catch(() => {});
-
-        const tx = await dapp.evaluate(settled("tx"));
-        const broadcast = node.broadcast.map((raw) => Transaction.from(raw));
-        check(
-            "the transaction approved after seven minutes reaches the site",
-            tx.settled === "resolved" &&
-                broadcast.length === 1 &&
-                tx.result === broadcast[0].hash &&
-                getAddress(broadcast[0].from) === address,
-            "the page got " +
-                JSON.stringify(tx) +
-                " and the node received " +
-                broadcast.length +
-                " transaction(s)",
-        );
-
-        const sign = await dapp.evaluate(settled("sign"));
-        check(
-            "the signature approved after seven minutes reaches the site",
-            sign.settled === "resolved" &&
-                getAddress(verifyMessage(getBytes(SIGN_HEX), sign.result)) ===
-                    address,
-            "the page got " + JSON.stringify(sign),
-        );
-    } catch (e) {
-        failed += 1;
-        console.log("# ERROR: " + (e && e.stack ? e.stack : e));
-    } finally {
-        if (browser.exitCode === null && browser.signalCode === null) {
-            const exited = new Promise((resolve) =>
-                browser.once("exit", resolve),
-            );
-            browser.kill();
-            await exited;
-        }
-        await node.close();
-        // Chromium's helper processes can still be writing to the profile
-        // for a moment after the browser process itself has exited.
-        fs.rmSync(profile, { recursive: true, force: true, maxRetries: 5 });
-    }
-
-    console.log("1.." + checks.length);
-    if (checks.length === 0) {
-        console.log("# FAILED: this program asserted nothing");
-        process.exitCode = 1;
-        return;
-    }
-    console.log(
-        "# " +
-            (checks.length - failed) +
-            "/" +
-            checks.length +
-            " checks passed",
-    );
-    if (failed > 0) {
-        console.log("# FAILED");
-        process.exitCode = 1;
-    }
-}
-
-main().catch((e) => {
-    console.error("slowApproval: " + (e && e.stack ? e.stack : e));
-    process.exitCode = 1;
-});

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/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) };
 }
@@ -312,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,
@@ -331,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
@@ -371,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.