From 8fcdd8a053e20de4c5c3bad4a3379446f2c85137 Mon Sep 17 00:00:00 2001 From: clawbot Date: Mon, 17 Aug 2026 09:34:07 +0200 Subject: [PATCH] fix: settle a site approval on the port that carries its teardown (closes #275) --- TODO.md | 12 ++ src/background/index.js | 81 ++++++--- src/popup/views/approval.js | 50 +++-- tests/backgroundApproval.test.js | 301 ++++++++++++++++++++++++++++++- tests/e2e/run.js | 136 ++++++++++---- 5 files changed, 509 insertions(+), 71 deletions(-) diff --git a/TODO.md b/TODO.md index 10ba0b5..5b3900f 100644 --- a/TODO.md +++ b/TODO.md @@ -225,6 +225,18 @@ but the review is broader than any of them. or broadcast failure — and were demonstrated failing first, the RPC one with `sendResponse` at zero calls ([#280](https://git.eeqj.de/sneak/AutistMask/issues/280)). +- 2026-08-14: Approving a site connection is no longer a race against the popup + closing. The decision now rides the approval port the popup already holds, + which is the same channel the close disconnects, so it is delivered ahead of + that disconnect however fast the teardown is; `windows.onRemoved` no longer + decides a site approval whose port is connected, since that event is ordered + against nothing either. Rejecting and closing without deciding both still + report a rejection, and the popup delays its own close by nothing. The e2e + harness's deferred-`window.close()` accommodation is gone with it, so the two + site-prompt tests now drive the shipped decide-then-close in a real Chromium; + against the unfixed code the approval came back to the page as + `{"settled":"rejected","code":4001}` + ([#275](https://git.eeqj.de/sneak/AutistMask/issues/275)). - 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js` rebuilt every failure as `new Error(error.message)`, so the code the background produced and the content script relayed intact was dropped in the diff --git a/src/background/index.js b/src/background/index.js index e4adfb4..0cbe61b 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -494,13 +494,53 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) { }); } -// Detect when an approval popup (browser-action) closes without a response. -// TX and sign approvals now use windows.create() and are handled by the -// windows.onRemoved listener below, but we still handle site-connection -// approval disconnects here. +// Anything only the extension's own pages may say. A content script speaks +// with the page's URL, so this is what separates the popup from the site the +// popup is being asked about. +function isExtensionSender(sender) { + const extUrl = runtime.getURL(""); + return !!(sender && sender.url && sender.url.startsWith(extUrl)); +} + +// The approval popup's port: it carries the user's decision on a +// site-connection approval, and its disconnect is how that approval learns the +// popup closed without one. +// +// The decision travels this port rather than a one-off runtime.sendMessage() +// for exactly one reason: the port is also what the popup's window.close() +// disconnects. A message posted on a port is delivered before that port's +// disconnect, so approve-then-close settles as an approval no matter how fast +// the teardown is. Sent as a one-off message the two crossed on independent +// channels with nothing ordering them, and the teardown won every time when +// the prompt was driven in a tab: the user approved and the dApp was told they +// had refused. +// +// TX and sign approvals do not decide here. They stay pending across a +// disconnect — the user can reopen the toolbar popup — and are rejected by the +// windows.onRemoved listener below. runtime.onConnect.addListener((port) => { if (port.name.startsWith("approval:")) { const id = port.name.split(":")[1]; + if (pendingApprovals[id] && isExtensionSender(port.sender)) { + // The extension's own popup is on the other end, so its disconnect + // is a trustworthy "closed" and onRemoved below stands down. The + // sender check is what keeps that from being an off switch: a + // content script that guessed the id and held its port open would + // otherwise disable the only settlement path a prompt whose popup + // never connected has left, and the dApp would wait forever. + pendingApprovals[id].portConnected = true; + } + port.onMessage.addListener((msg) => { + if (!msg || msg.type !== "AUTISTMASK_APPROVAL_DECISION") return; + if (!isExtensionSender(port.sender)) return; + const approval = pendingApprovals[id]; + if (!approval || approval.type === "tx" || approval.type === "sign") + return; + settleApproval(id, { + approved: !!msg.approved, + remember: !!msg.remember, + }); + }); port.onDisconnect.addListener(() => { const approval = pendingApprovals[id]; if (approval) { @@ -510,7 +550,6 @@ runtime.onConnect.addListener((port) => { } settleApproval(id, { approved: false, remember: false }); } - resetPopupUrl(); }); } }); @@ -1074,10 +1113,21 @@ startBackgroundJobs(); // outcome to the page — and the window is recorded as gone, so that an attempt // which then fails retryably settles instead of waiting in a window that no // longer exists. +// +// A site-connection approval whose popup connected its port is not decided +// here. That popup approves and closes in the same breath, and this event +// races the decision on a channel of its own — the same race the port exists +// to end. Its port disconnect says the same thing this event does, in an order +// that is defined, so the disconnect is left to say it. The window closing +// before any port connected is the one case with nothing else to speak for it, +// and is rejected here so the dApp is not left waiting on a window that is +// gone. if (windowsNs && windowsNs.onRemoved) { windowsNs.onRemoved.addListener((windowId) => { for (const [id, approval] of Object.entries(pendingApprovals)) { if (approval.windowId !== windowId) continue; + const isSite = approval.type !== "tx" && approval.type !== "sign"; + if (isSite && approval.portConnected) continue; const rejection = abandonedResult( approval, APPROVAL_REJECTED_CODE, @@ -1127,18 +1177,16 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { } // Validate that popup-only messages originate from the extension itself. + // The site-connection decision is not here: it is a port message, and it + // is checked the same way where the port is served. const POPUP_ONLY_TYPES = [ "AUTISTMASK_GET_APPROVAL", - "AUTISTMASK_APPROVAL_RESPONSE", "AUTISTMASK_TX_RESPONSE", "AUTISTMASK_SIGN_RESPONSE", ]; - if (POPUP_ONLY_TYPES.includes(msg.type)) { - const extUrl = runtime.getURL(""); - if (!sender.url || !sender.url.startsWith(extUrl)) { - sendResponse({ error: "Unauthorized sender" }); - return false; - } + if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) { + sendResponse({ error: "Unauthorized sender" }); + return false; } if (msg.type === "AUTISTMASK_GET_APPROVAL") { @@ -1170,15 +1218,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { return false; } - if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") { - settleApproval(msg.id, { - approved: msg.approved, - remember: msg.remember, - }); - resetPopupUrl(); - return false; - } - if (msg.type === "AUTISTMASK_TX_RESPONSE") { const approval = pendingApprovals[msg.id]; if (!approval) return false; diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 4f03f48..167a9a5 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -443,7 +443,7 @@ function showSignApproval(details) { // describe the approval is the same outcome as an approval that is gone. async function show(id) { approvalId = id; - runtimeApi().connect({ name: "approval:" + id }); + approvalPort = runtimeApi().connect({ name: "approval:" + id }); let details = null; try { @@ -476,6 +476,14 @@ async function show(id) { } let approvalId = null; +// The port this approval was opened on. Closing this window disconnects it, +// and the background treats that disconnect as "closed without deciding" for a +// site connection — so the decision goes out on this same port and not as a +// one-off message. One channel is ordered: a message posted on it is delivered +// before its own disconnect, however immediately the close follows. Two +// channels were not, and the close won, reporting a user who approved as +// having refused. +let approvalPort = null; let pendingTxDetails = null; // The exact objects shown to the user, kept so the popup signs what it // displayed rather than re-fetching or re-populating anything at approval @@ -543,6 +551,28 @@ function clearSignPassword() { hideError("approve-sign-error"); } +// Answer a site-connection approval and close. The decision goes out on the +// approval port — see approvalPort above for why — and carries no approval id, +// because the port name already names the approval the background will settle. +// The post is guarded because a throw must not cost the close: posting on a +// port whose background worker has been torn down throws, and the approval it +// would have settled died with that worker, so the only thing left to do is +// what the user asked for — go away. +function decideSite(approved) { + if (approvalPort) { + try { + approvalPort.postMessage({ + type: "AUTISTMASK_APPROVAL_DECISION", + approved, + remember: $("approve-remember").checked, + }); + } catch { + // Nothing to report it to; the window closes either way. + } + } + window.close(); +} + function init(_ctx) { onViewLeave("approve-tx", clearTxPassword); onViewLeave("approve-sign", clearSignPassword); @@ -553,25 +583,11 @@ function init(_ctx) { }); $("btn-approve").addEventListener("click", () => { - const remember = $("approve-remember").checked; - notify({ - type: "AUTISTMASK_APPROVAL_RESPONSE", - id: approvalId, - approved: true, - remember, - }); - window.close(); + decideSite(true); }); $("btn-reject").addEventListener("click", () => { - const remember = $("approve-remember").checked; - notify({ - type: "AUTISTMASK_APPROVAL_RESPONSE", - id: approvalId, - approved: false, - remember, - }); - window.close(); + decideSite(false); }); $("btn-approve-tx").addEventListener("click", async () => { diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index 464bf38..1e2b33f 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -39,6 +39,21 @@ const HOSTNAME = "dapp.example"; const UNCONNECTED_ORIGIN = "https://stranger.example"; const EXT_URL = "chrome-extension://autistmask/"; +// An origin the persisted state has never allowed, so asking to connect from +// it raises a prompt rather than being answered from allowedSites. +const FRESH_ORIGIN = "https://fresh.example"; + +// The approval id in the most recent popup URL of a list, or null when none +// of them carries one. Takes both shapes: the absolute URL windows.create() +// is given and the extension-relative one action.setPopup() is given. +function approvalIdIn(urls) { + for (let i = urls.length - 1; i >= 0; i--) { + if (!urls[i] || !urls[i].includes("?approval=")) continue; + return new URL(urls[i], EXT_URL).searchParams.get("approval"); + } + return null; +} + // What the dApp asks for: no nonce, no gas, no fees. This is the shape that // makes a duplicate broadcast possible at all. const TX_PARAMS = { @@ -173,8 +188,13 @@ function loadBackground(options) { let messageListener = null; let windowRemovedListener = null; + let connectListener = null; const created = []; const removed = []; + // Every URL the background put on the browser action. A site approval + // raised through action.openPopup() opens no window at all, so this is + // the only place its id appears. + const actionPopups = []; global.chrome = { storage: { @@ -193,7 +213,14 @@ function loadBackground(options) { messageListener = fn; }, }, - onConnect: { addListener: () => {} }, + // Captured, not swallowed: the approval port is what carries a + // site connection's decision and the popup teardown that races + // it, so a no-op stub here hides the whole subject of #275. + onConnect: { + addListener: (fn) => { + connectListener = fn; + }, + }, lastError: null, }, windows: { @@ -221,7 +248,17 @@ function loadBackground(options) { query: (q, cb) => cb([]), sendMessage: () => {}, }, - action: { setPopup: () => {} }, + action: { + setPopup: (o) => { + actionPopups.push(o.popup); + }, + // The production route for a site connection. Present only when + // a test asks for it, because with it the prompt is the toolbar + // popup: no window is created, so windows.onRemoved can never + // fire for it and the port disconnect is the only close signal + // that exists. + ...(opts.actionPopup ? { openPopup: () => Promise.resolve() } : {}), + }, }; require("../src/background/index"); @@ -289,6 +326,70 @@ function loadBackground(options) { }; } + // A dApp asking to connect. The origin defaults to one the persisted + // state has never allowed, so the request really does raise a prompt + // instead of being answered from allowedSites. + function requestSite(origin) { + let rpcResult = null; + messageListener( + { + type: "AUTISTMASK_RPC", + method: "eth_requestAccounts", + params: [], + }, + { origin: origin || FRESH_ORIGIN }, + (r) => { + rpcResult = r; + }, + ); + return { + // Wherever the prompt went: the toolbar popup URL when + // action.openPopup() carried it, the created window otherwise. + id: () => + approvalIdIn(actionPopups) || + approvalIdIn(created.map((c) => c.url)), + result: () => rpcResult, + }; + } + + // The popup's approval port, as the browser delivers it. Messages posted + // on a port and that port's disconnect travel one channel in FIFO order, + // which is exactly the property the fix rests on, so this stub delivers + // them in the order the caller emits them and never reorders them. + function connectApproval(id, senderUrl) { + const onMessage = []; + const onDisconnect = []; + const port = { + name: "approval:" + id, + sender: { + url: + senderUrl === undefined + ? EXT_URL + "src/popup/index.html?approval=" + id + : senderUrl, + }, + onMessage: { addListener: (fn) => onMessage.push(fn) }, + onDisconnect: { addListener: (fn) => onDisconnect.push(fn) }, + }; + connectListener(port); + return { + decide: (approved, remember) => { + for (const fn of onMessage) { + fn( + { + type: "AUTISTMASK_APPROVAL_DECISION", + approved, + remember: !!remember, + }, + port, + ); + } + }, + disconnect: () => { + for (const fn of onDisconnect) fn(port); + }, + }; + } + // The user closes the approval popup. `created` is index-aligned with the // ids the window stub hands back, so window 1 is the first popup opened. function closeWindow(windowId) { @@ -299,6 +400,8 @@ function loadBackground(options) { send, requestTx, requestSign, + requestSite, + connectApproval, closeWindow, broadcastTransaction, loadState, @@ -1618,3 +1721,197 @@ describe("popup-only messages", () => { }); }); }); + +// A site connection decided in a popup that closes on the next line. +// +// The decision and the teardown are two events the popup emits back to back, +// and the background must not be able to reach different outcomes depending on +// which of them it processes first. It cannot, because they are now one +// channel: the decision is posted on the approval port that the close then +// disconnects, so it is delivered first. Every test here therefore emits the +// close IMMEDIATELY after the decision, with nothing awaited in between — +// which is what the popup does, and what used to report a user who approved as +// having refused (#275). +describe("a site connection decided as the popup closes", () => { + // The production route: chrome.action.openPopup() put the prompt in the + // toolbar popup, which is not a window, so nothing but the port + // disconnect can tell the background this prompt is gone. + test("approving in the toolbar popup connects the site", async () => { + const bg = loadBackground({ actionPopup: true }); + const pending = bg.requestSite(); + await settle(); + const id = pending.id(); + expect(id).toBeTruthy(); + expect(bg.created).toHaveLength(0); + + const port = bg.connectApproval(id); + port.decide(true, false); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ result: [signer.address] }); + }); + + test("closing the toolbar popup without deciding is a rejection", async () => { + const bg = loadBackground({ actionPopup: true }); + const pending = bg.requestSite(); + await settle(); + + const port = bg.connectApproval(pending.id()); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + test("rejecting is a rejection, and the close that follows adds nothing", async () => { + const bg = loadBackground({ actionPopup: true }); + const pending = bg.requestSite(); + await settle(); + + const port = bg.connectApproval(pending.id()); + port.decide(false, false); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + // The port carries a decision now, so it carries the sender check the + // one-off message used to carry. A content script that guessed an + // approval id must not be able to connect the site it is running on. + test("a decision from a page sender is ignored, and the close rejects", async () => { + const bg = loadBackground({ actionPopup: true }); + const pending = bg.requestSite(); + await settle(); + + const port = bg.connectApproval(pending.id(), FRESH_ORIGIN + "/x.html"); + port.decide(true, true); + await settle(); + expect(pending.result()).toBeNull(); + + port.disconnect(); + await settle(); + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + // The fallback shape, where openPopup() is unavailable and the prompt is + // a window the extension opened. Closing it fires windows.onRemoved as + // well, on a channel of its own that is ordered against nothing — so the + // window event must not be allowed to decide a site approval either. + // + // The event goes FIRST here, which is the interleaving the guard in the + // onRemoved listener exists for: the approval is still pending when the + // event arrives, so the listener really reaches it and really has to + // decline it. With the decision first there is nothing left in + // pendingApprovals and the listener finds no approval to spare. + test("approving in the fallback window survives a window event that lands first", async () => { + const bg = loadBackground(); + const pending = bg.requestSite(); + await settle(); + expect(bg.created).toHaveLength(1); + + const port = bg.connectApproval(pending.id()); + bg.closeWindow(1); + port.decide(true, false); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ result: [signer.address] }); + }); + + test("approving in the fallback window survives a window event that follows", async () => { + const bg = loadBackground(); + const pending = bg.requestSite(); + await settle(); + + const port = bg.connectApproval(pending.id()); + port.decide(true, false); + bg.closeWindow(1); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ result: [signer.address] }); + }); + + // The connected port is what silences the window event, so connecting one + // must take the same sender check the decision takes. Otherwise a content + // script that guessed the id switches off the only settlement path a + // prompt whose real popup never connected has, and the dApp hangs. + test("a port from a page sender does not silence the window event", async () => { + const bg = loadBackground(); + const pending = bg.requestSite(); + await settle(); + + // Connected and held open — no disconnect, so nothing but the window + // event can settle this approval. + bg.connectApproval(pending.id(), FRESH_ORIGIN + "/x.html"); + bg.closeWindow(1); + await settle(); + + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + // Same shape, and the same window event arriving before the popup has + // said anything at all — which is a user closing the window rather than + // deciding, and still has to reach the dApp as a rejection. + test("closing the fallback window without deciding is a rejection", async () => { + const bg = loadBackground(); + const pending = bg.requestSite(); + await settle(); + + const port = bg.connectApproval(pending.id()); + bg.closeWindow(1); + port.disconnect(); + await settle(); + + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + // The net under the paragraph above: a prompt whose page never got as far + // as connecting the port has no disconnect to reject it, so the window + // event has to. Otherwise the dApp waits forever on a window that is gone. + test("a window that closes before its popup ever connected still rejects", async () => { + const bg = loadBackground(); + const pending = bg.requestSite(); + await settle(); + + bg.closeWindow(1); + await settle(); + + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); + + // The `isSite &&` half of that skip, which is what keeps it from reaching + // a tx or sign approval. The popup connects its port in show() before it + // knows the approval's type, and the background sets portConnected without + // looking at the type either, so a tx approval in the fallback window + // carries the flag too. Without the conjunct the window event would skip + // it, windowClosed would never be set, releaseApproval() would never settle + // it, and the page would hang — the #271 regression this guard is written + // around. + test("a tx window closed with the port connected still rejects", async () => { + const bg = loadBackground(); + const pending = bg.requestTx(); + await settle(); + bg.connectApproval(pending.id()); + await settle(); + bg.closeWindow(1); + await settle(); + expect(pending.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + }); +}); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 3d14ace..855f633 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -2183,32 +2183,13 @@ async function reserveApprovalTab(env) { // one down with it. env.approvalTab = await env.ctx.newPage(); - // The one accommodation this section makes to the shipped code, and the - // reason for it. - // - // Both approval buttons call runtime.sendMessage() and then window.close() - // on the next line. Closing this page disconnects the approval port, and - // the disconnect handler in src/background/index.js settles a pending - // site approval as a rejection. In a tab those two race and the teardown - // wins: the approve message is never acted on, and the page is told the - // user rejected. Measured — with the close left in place the approval - // resolves as a rejection every time; with it deferred it resolves as an - // approval every time. - // - // It is deferred, not removed: the harness closes the page itself once - // the outcome has been observed, which is what window.close() would have - // done, only after the message it was racing has been processed. - // - // This affects the site-connection prompt only. The sign and transaction - // prompts run in windows the extension opens itself, with window.close() - // untouched, and their disconnect handler deliberately keeps a tx or sign - // approval pending rather than rejecting it — so there is no race there - // to accommodate. Whether the same ordering holds in a real toolbar popup - // is not observable from a headless harness and is reported rather than - // assumed either way. - await env.approvalTab.addInitScript(() => { - window.close = function () {}; - }); + // This tab runs the shipped popup with nothing patched. The site + // approval buttons decide and then close on the next line, and the two + // site-approval tests below are therefore the real-browser + // approve-then-immediate-close and reject-then-immediate-close cases: the + // decision rides the approval port, which also carries the disconnect the + // close causes, so it is delivered ahead of it and the outcome does not + // depend on the teardown timing (#275). await env.approvalTab.goto("about:blank"); await sleep(APPROVAL_TAB_SETTLE_MS); return env.approvalTab; @@ -2257,6 +2238,95 @@ async function closeApprovalPages(ctx) { } } +// Click a button whose own handler closes the window it lives in — every +// Reject, and Allow on the site prompt. +// +// page.click() dispatches the click and then waits for the renderer to +// acknowledge it, and a page torn down by the handler never gets to. The +// dispatch is what the test needs and the log shows it happening ("performing +// click action") immediately before the failure; the page going away is the +// button working, not the click failing. Observed on #btn-reject-sign and +// #btn-reject-tx, whose windows have always closed themselves. +// +// What the swallow costs is not the same for every button, so neither is what +// proves the click landed: +// +// #btn-reject-sign, #btn-reject-tx — their disconnect leaves the approval +// pending, so a click that never landed leaves the dApp promise unsettled +// and the assertion after the call fails on its own. +// #btn-approve — only a decision resolves the promise, and a swallowed click +// cannot produce settled === "resolved". +// #btn-reject on the site prompt — NOT self-proving. A page that went away +// without the click landing disconnects the approval port, the background +// settles that as 4001, and 4001 is exactly what assertUserRejection +// accepts. That call site arms the click trace below and asserts it. +// +// A button that is missing or unclickable raises a different error, which is +// rethrown. +async function clickAndClose(page, selector) { + try { + await page.click(selector); + } catch (e) { + if (!String((e && e.message) || e).includes("has been closed")) throw e; + } +} + +// Evidence that a click reached the button, for the button whose outcome +// cannot tell. +// +// A capture-phase listener on the document runs ahead of the button's own +// handler and writes one key with localStorage.setItem(), which is synchronous +// and therefore already in the browser process when the handler tears the page +// down a line later. Any other page of the extension origin can read it back, +// and env.page is one. The listener only observes: nothing about the shipped +// decide-then-close is deferred, patched or reordered. +const CLICK_TRACE_KEY = "autistmask-e2e-click-landed"; + +async function armClickTrace(env, page, selector) { + await env.page.evaluate( + (key) => localStorage.removeItem(key), + CLICK_TRACE_KEY, + ); + await page.evaluate( + ({ key, sel }) => { + document.addEventListener( + "click", + (e) => { + const target = e.target; + if (target && target.closest && target.closest(sel)) { + localStorage.setItem(key, sel); + } + }, + true, + ); + }, + { key: CLICK_TRACE_KEY, sel: selector }, + ); +} + +// The write crosses processes to reach env.page's renderer, so it is waited +// for rather than read once. Nothing else in the test is timed on this. +async function assertClickLanded(env, selector, timeout = 5000) { + const deadline = Date.now() + timeout; + let seen; + for (;;) { + seen = await env.page.evaluate( + (key) => localStorage.getItem(key), + CLICK_TRACE_KEY, + ); + if (seen === selector || Date.now() > deadline) break; + await sleep(25); + } + assert( + seen === selector, + "the click on " + + selector + + " never reached the button, so the outcome below proves nothing " + + "about it: trace was " + + JSON.stringify(seen), + ); +} + // Record every message the approval window sends to the background worker. // // This is the direct observation the password check needs. It is installed @@ -2477,7 +2547,11 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as // origin in deniedSites and every later test in this section is // auto-rejected with no prompt at all, which would look like a pass. await popup.uncheck("#approve-remember"); - await popup.click("#btn-reject"); + // The rejection this asserts is also what an unclicked prompt that + // simply went away produces, so the click itself is witnessed. + await armClickTrace(env, popup, "#btn-reject"); + await clickAndClose(popup, "#btn-reject"); + await assertClickLanded(env, "#btn-reject"); await assertUserRejection( env.dapp, @@ -2508,7 +2582,7 @@ test("eth_requestAccounts approved returns the selected address (#183)", async ( // does not, and the sign and transaction tests below all require the // origin to still be authorized. await popup.check("#approve-remember"); - await popup.click("#btn-approve"); + await clickAndClose(popup, "#btn-approve"); outcome = await settleRequest(env.dapp, "accounts"); } finally { @@ -2616,7 +2690,7 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env ]); const popup = await waitForApprovalWindow(env.ctx); await visible(popup, "#view-approve-sign"); - await popup.click("#btn-reject-sign"); + await clickAndClose(popup, "#btn-reject-sign"); await assertUserRejection( env.dapp, @@ -2719,7 +2793,7 @@ test("eth_signTypedData_v4 rejected returns a rejection to the page (#183)", asy ]); const popup = await waitForApprovalWindow(env.ctx); await visible(popup, "#view-approve-sign"); - await popup.click("#btn-reject-sign"); + await clickAndClose(popup, "#btn-reject-sign"); await assertUserRejection( env.dapp, @@ -2877,7 +2951,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => { ]); const popup = await waitForApprovalWindow(env.ctx); await visible(popup, "#view-approve-tx"); - await popup.click("#btn-reject-tx"); + await clickAndClose(popup, "#btn-reject-tx"); await assertUserRejection( env.dapp,