docs: say what a closed window does to a tx or sign approval (closes #505)

The approval port's disconnect handler leaves a transaction or signing
approval pending, and its comments said that was so the user could reopen
the toolbar popup. Those approvals open in their own window. When that
window closes, the windows.onRemoved listener rejects the approval, unless
an approved attempt is still running, in which case that attempt answers
the page. Both comments now say so. No code changed.

Model: opus-5-5

README.md

@@ -239,12 +239,8 @@ 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, 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/fmt` — format all files (writes), on the host
+- `script/fmt-check` — check formatting (read-only), on the host
 - `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
@@ -347,12 +343,6 @@ 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, and removes that
-image when it exits, an interrupt included. Two clones can therefore run the
-same suite at once without one build taking the other's image away, and no run
-leaves an image behind.
-
 ### Chrome (`make test-e2e`)
 
 `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
@@ -2170,13 +2160,6 @@ 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
@@ -2669,8 +2652,7 @@ 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), and checks a hostname written with a
-trailing dot (`name.`) as `name`.
+parent domains (subdomain matching).
 
 #### Transaction Decoding
 

script/fmt

@@ -25,13 +25,6 @@ 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,13 +26,6 @@ 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,18 +22,13 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-# 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.
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome:$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
+
+IIDFILE=""
 
 cleanup() {
-    # 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
+    if [ -n "$IIDFILE" ]; then
+        rm -f "$IIDFILE"
     fi
 }
 
@@ -45,15 +40,22 @@ main() {
         exit 1
     fi
 
+    IIDFILE="$(mktemp)"
     trap cleanup EXIT
     trap 'cleanup; exit 130' 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 -t "$IMAGE" -f tests/e2e/Dockerfile .
+    docker build --no-cache --iidfile "$IIDFILE" -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
@@ -77,7 +79,7 @@ main() {
         -e HOME=/tmp \
         -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
         -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/run.js
 
     # Where chrome.storage.local lives, and what moves it: two unpacked loads
@@ -93,7 +95,7 @@ main() {
     docker run --rm \
         --ipc=host \
         -e HOME=/tmp \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/storagePartition.js
 }
 

script/test-e2e-firefox

@@ -21,13 +21,13 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-# A tag of this run's own, as in script/test-e2e, which says why.
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox:$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
+
+IIDFILE=""
 
 cleanup() {
-    # 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
+    if [ -n "$IIDFILE" ]; then
+        rm -f "$IIDFILE"
     fi
 }
 
@@ -39,15 +39,22 @@ main() {
         exit 1
     fi
 
+    IIDFILE="$(mktemp)"
     trap cleanup EXIT
     trap 'cleanup; exit 130' 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 -t "$IMAGE" -f tests/e2e/firefox/Dockerfile .
+    docker build --no-cache --iidfile "$IIDFILE" -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
@@ -65,7 +72,7 @@ main() {
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/firefox/run.js dist/firefox
 
     # The install/uninstall/re-install property, against the packaged XPI
@@ -78,7 +85,7 @@ main() {
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/firefox/reinstall.js
 }
 

src/background/index.js

@@ -22,7 +22,6 @@ const { debugFetch, log } = require("../shared/log");
 const {
     verifySignedTx,
     verifySignature,
-    typedDataChainRefusal,
     failureIsRetryable,
     describeTxFailure,
     sameAddress,
@@ -1768,8 +1767,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
 
         (async () => {
             try {
-                const s = await getState();
-                const activeAddress = activeAddressOf(s);
+                const activeAddress = await getActiveAddress();
                 // 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.
@@ -1788,20 +1786,6 @@ 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, currentNetwork } = require("../../shared/state");
+const { state, saveState } = require("../../shared/state");
 const {
     networkById,
     networkByChainId,
@@ -46,10 +46,7 @@ const {
 const { decryptWithPassword } = require("../../shared/vault");
 const { getSignerForAddress } = require("../../shared/wallet");
 const { walletDefect } = require("../../shared/walletDefects");
-const {
-    describeSigningFailure,
-    typedDataChainRefusal,
-} = require("../../shared/approvalVerify");
+const { describeSigningFailure } = require("../../shared/approvalVerify");
 const txStatus = require("./txStatus");
 const uniswap = require("../../shared/uniswap");
 const { notify, runtimeApi, sendMessage } = require("../../shared/browserApi");
@@ -446,8 +443,7 @@ 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. Its domain may name no chain but the active
-// network's.
+// message while another is signed.
 function typedDataRefusal(sp) {
     if (sp.method === "personal_sign" || sp.method === "eth_sign") return null;
     let data;
@@ -470,7 +466,7 @@ function typedDataRefusal(sp) {
             ", so it cannot be signed."
         );
     }
-    return typedDataChainRefusal(data.domain, currentNetwork());
+    return null;
 }
 
 // The largest amount a Permit2 allowance can hold, a uint160. Permit2 treats

src/shared/approvalVerify.js

@@ -53,7 +53,6 @@ const {
     accessListify,
     formatEther,
     getAddress,
-    getBigInt,
     getBytes,
     toQuantity,
     verifyMessage,
@@ -636,33 +635,6 @@ 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";
@@ -824,7 +796,6 @@ function describeSigningFailure(response, fallbackMessage) {
 module.exports = {
     verifySignedTx,
     verifySignature,
-    typedDataChainRefusal,
     assertNoForbiddenFields,
     assertNothingUnchecked,
     assertCanonicalBytes,

src/shared/phishingDomains.js

@@ -110,14 +110,11 @@ 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().replace(/\.$/, "");
+    const h = hostname.toLowerCase();
     const variants = [h];
     const parts = h.split(".");
     // Parent domains: a.b.c.d -> b.c.d, c.d

src/shared/walletDefects.js

@@ -36,13 +36,11 @@ 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, 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.",
+            "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.",
             "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/backgroundApproval.test.js

@@ -397,13 +397,7 @@ function loadBackground(options) {
                 rpcResult = r;
             },
         );
-        return {
-            id: () =>
-                new URL(created[created.length - 1].url).searchParams.get(
-                    "approval",
-                ),
-            result: () => rpcResult,
-        };
+        return { result: () => rpcResult };
     }
 
     // A dApp asking to connect. The origin defaults to one the persisted
@@ -1490,55 +1484,6 @@ 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);

tests/phishingDomains.test.js

@@ -89,12 +89,6 @@ 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);
@@ -139,13 +133,6 @@ 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/typedDataPermit.test.js

@@ -240,7 +240,6 @@ function request(data) {
 beforeEach(() => {
     state.trackedTokens = [];
     state.wallets = [];
-    state.networkId = "mainnet";
 });
 
 describe("a token permission is warned about, naming spender and amount", () => {
@@ -533,66 +532,3 @@ 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,18 +153,6 @@ 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(" ");