fix: open no approval window for a site-connection prompt already answered (closes #287)
check / check (push) Failing after 3s
e2e / e2e-chrome (push) Failing after 3s
e2e / e2e-firefox (push) Failing after 3s

When a site-connection prompt was decided before the toolbar popup raised
for it had loaded, that popup was torn down, chrome.action.openPopup()
rejected, and the background opened its fallback window for the answered
approval and only then removed it. In the Chrome end-to-end suite the next
test could take that window for its own prompt and lose it under its wait.
openApprovalWindow() now returns before creating a window when the approval
is no longer pending.

The blocklist test clicked its self-closing Reject with a plain click; it
now clicks it as the other site Reject does, with the click witnessed.
README.md and the e2e workflow comment no longer name this issue as what
keeps e2e-chrome from being a required check.

Model: opus-5-5
This commit was merged in pull request #444.
This commit is contained in:
2026-10-05 04:09:07 +02:00
parent 8c8caafe33
commit 18bdafd130
6 changed files with 57 additions and 15 deletions
+21
View File
@@ -2235,6 +2235,27 @@ describe("a site connection decided as the popup closes", () => {
});
});
// The prompt is decided before the toolbar popup raised for it has
// loaded; that popup is torn down and openPopup() rejects only after.
test("a toolbar prompt already decided opens no window when openPopup() rejects", async () => {
const bg = loadBackground({ actionPopup: true });
const opening = deferred();
bg.openPopup.mockImplementation(() => opening.promise);
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
opening.reject(new Error("the toolbar popup closed before it loaded"));
await settle();
expect(bg.created).toHaveLength(0);
});
// 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.
+4 -2
View File
@@ -2738,7 +2738,7 @@ async function closeApprovalPages(ctx) {
// #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.
// accepts. Both call sites arm the click trace below and assert it.
//
// A button that is missing or unclickable raises a different error, which is
// rethrown.
@@ -3124,7 +3124,9 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
// Not remembered: a remembered decision for this origin would
// outlive the test.
await popup.uncheck("#approve-remember");
await popup.click("#btn-reject");
await armClickTrace(env, popup, "#btn-reject");
await clickAndClose(popup, "#btn-reject");
await assertClickLanded(env, "#btn-reject");
await assertUserRejection(
phishingDapp,