Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
af757eb552 | ||
|
|
6613a1f6bc |
@@ -381,11 +381,13 @@ handler on a reserved-TLD origin, gets `window.ethereum` from the shipped
|
||||
the runner and compared against the active address, the transaction assertions
|
||||
run against the raw signed transaction captured at `eth_sendRawTransaction`
|
||||
rather than against anything the extension reported, rejecting each prompt is
|
||||
required to return a rejection to the page rather than hang or resolve, and the
|
||||
password is required to be absent from every message the approval window sends
|
||||
to the background — with the message that would carry it required to be present,
|
||||
so that check cannot pass by observing nothing. That last one is the standing
|
||||
floor under [#157](https://git.eeqj.de/sneak/AutistMask/issues/157).
|
||||
required to return a rejection to the page rather than hang or resolve, a prompt
|
||||
raised while another approval window has focus is required to open a window of
|
||||
its own, and the password is required to be absent from every message the
|
||||
approval window sends to the background — with the message that would carry it
|
||||
required to be present, so that check cannot pass by observing nothing. That
|
||||
last one is the standing floor under
|
||||
[#157](https://git.eeqj.de/sneak/AutistMask/issues/157).
|
||||
|
||||
Two limits of that coverage, neither of them papered over. The RPC is stubbed
|
||||
throughout, so this is **not** a real dApp against a real network with real
|
||||
@@ -619,12 +621,9 @@ The jobs **report, they do not gate.** A failure is a red mark against the
|
||||
commit that a reviewer has to account for, not a hard block: whether a check
|
||||
blocks a merge is Gitea branch protection, which this repo does not configure.
|
||||
|
||||
That is not only a statement about configuration. A report of the Chrome suite
|
||||
**failing under load** is still open:
|
||||
[#290](https://git.eeqj.de/sneak/AutistMask/issues/290), runs on a busy machine
|
||||
failing with `the extension opened no approval window within 30000ms`. So a red
|
||||
`e2e-chrome` has to be read before it is believed, and those failures are the
|
||||
blocker to ever making this a required check. Do not answer them with a retry
|
||||
That is not only a statement about configuration. No report of the Chrome suite
|
||||
**failing under load** is open now, but it has failed that way before, so a red
|
||||
`e2e-chrome` is read before it is believed. Do not answer one with a retry
|
||||
wrapper: a suite that reruns until it is green stops being evidence.
|
||||
|
||||
Nothing in either job can pass vacuously. There is no `continue-on-error` and no
|
||||
@@ -1960,7 +1959,9 @@ view would leave a wallet one click from deletion.
|
||||
`eth_sendTransaction` arriving while one is unanswered is refused with
|
||||
EIP-1193 code `-32002` rather than being populated at the same nonce. It opens
|
||||
no window and takes no nonce, and the site can send it again once the pending
|
||||
one is answered.
|
||||
one is answered. The window is centred on the browser window the user was last
|
||||
in; if that was another approval window, or the browser refuses the centred
|
||||
position, the browser picks the position.
|
||||
- **Elements**:
|
||||
- "Transaction Request" heading
|
||||
- Phishing warning banner (shown when the hostname is on the phishing
|
||||
|
||||
@@ -58,6 +58,20 @@ but the review is broader than any of them.
|
||||
`README.md` says. `addressColor()` and `etherscanAddressUrl()` are no longer
|
||||
exported. Nothing the user sees changed.
|
||||
|
||||
- 2026-10-05: A prompt raised while another approval window has focus opens a
|
||||
window of its own ([#290](https://git.eeqj.de/sneak/AutistMask/issues/290)).
|
||||
The background centred each approval window on the last focused window, which
|
||||
could be an earlier approval window still open; headless Chrome reports one as
|
||||
1280x720, so the new window came out where the browser refused to create it,
|
||||
and the request failed with no window at all. It now centres only on a browser
|
||||
window, and when the browser refuses the position it asks again without one
|
||||
and lets the browser place the window. In the Chrome end-to-end suite a test
|
||||
could raise its prompt while the previous test's window was still closing, and
|
||||
then either hit that refusal or take the closing window for its own. After a
|
||||
test that passed, the runner now waits a few seconds for approval windows to
|
||||
close and fails the test if one is still open; after a test that failed, it
|
||||
closes them.
|
||||
|
||||
- 2026-10-05: The Send screen has a "Max" button
|
||||
([#198](https://git.eeqj.de/sneak/AutistMask/issues/198)). Emptying an ETH
|
||||
address took guessing an amount and being refused by the confirmation screen's
|
||||
|
||||
+22
-3
@@ -437,7 +437,13 @@ async function openApprovalWindow(id) {
|
||||
width: popupWidth,
|
||||
height: popupHeight,
|
||||
};
|
||||
if (currentWin) {
|
||||
// Centred on a browser window only. The last focused window can be
|
||||
// another approval window still open, and centring on one can give a
|
||||
// position the browser refuses ("Bounds must be at least 50% within
|
||||
// visible screen space"): headless Chrome reports this 360x600 popup as
|
||||
// 1280x720. The request then failed with no window at all. Over a popup,
|
||||
// the browser picks the position.
|
||||
if (currentWin && currentWin.type === "normal") {
|
||||
opts.left = Math.round(
|
||||
currentWin.left + (currentWin.width - popupWidth) / 2,
|
||||
);
|
||||
@@ -455,10 +461,23 @@ async function openApprovalWindow(id) {
|
||||
win = await windowsCreate(opts);
|
||||
} catch (e) {
|
||||
// The promise namespace reports the failure by rejecting where the
|
||||
// callback namespace reported it by handing back no window; both land
|
||||
// on the !win branch below, which settles the approval.
|
||||
// callback namespace reported it by handing back no window; both
|
||||
// leave win null.
|
||||
log.errorf("could not open the approval window:", e);
|
||||
}
|
||||
// The browser also refuses a centred position that is too far off screen,
|
||||
// as it is over a browser window near the screen edge. Asked again
|
||||
// without a position, it places the window itself. If that fails too,
|
||||
// the !win branch below settles the approval.
|
||||
if (!win && opts.left !== undefined) {
|
||||
delete opts.left;
|
||||
delete opts.top;
|
||||
try {
|
||||
win = await windowsCreate(opts);
|
||||
} catch (e) {
|
||||
log.errorf("could not open the approval window:", e);
|
||||
}
|
||||
}
|
||||
|
||||
const approval = pendingApprovals[id];
|
||||
if (!approval) {
|
||||
|
||||
@@ -267,9 +267,22 @@ function loadBackground(options) {
|
||||
lastError: null,
|
||||
},
|
||||
windows: {
|
||||
getLastFocused: (cb) => cb(null),
|
||||
getLastFocused: (cb) => cb(opts.lastFocused || null),
|
||||
create: (options2, cb) => {
|
||||
created.push(options2);
|
||||
// A copy, as the browser takes it at the call: the background
|
||||
// reuses the object when it asks a second time.
|
||||
created.push({ ...options2 });
|
||||
// A browser that refuses any position it is given, as Chrome
|
||||
// does for one it judges too far off screen.
|
||||
if (opts.refusePosition && options2.left !== undefined) {
|
||||
global.chrome.runtime.lastError = {
|
||||
message:
|
||||
"Invalid value for bounds. Bounds must be at least 50% within visible screen space.",
|
||||
};
|
||||
cb(undefined);
|
||||
global.chrome.runtime.lastError = null;
|
||||
return;
|
||||
}
|
||||
// A browser that answers with no window at all. The approval
|
||||
// then has no window it can ever be answered in.
|
||||
cb(opts.noWindow ? undefined : { id: created.length });
|
||||
@@ -2716,3 +2729,77 @@ describe("removing a site in Settings disconnects it", () => {
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [signer.address] });
|
||||
});
|
||||
});
|
||||
|
||||
// An approval window still open is often the last focused window, and headless
|
||||
// Chrome reports one as 1280x720. Centred on that, the next approval window
|
||||
// lands where the browser refuses to create it, and its request failed with no
|
||||
// window at all (https://git.eeqj.de/sneak/AutistMask/issues/290).
|
||||
describe("where an approval window opens", () => {
|
||||
test("centred on the browser window the user was last in", async () => {
|
||||
const bg = loadBackground({
|
||||
lastFocused: {
|
||||
type: "normal",
|
||||
left: 0,
|
||||
top: 0,
|
||||
width: 1280,
|
||||
height: 720,
|
||||
},
|
||||
});
|
||||
|
||||
bg.requestSign();
|
||||
await settle();
|
||||
|
||||
expect(bg.created).toHaveLength(1);
|
||||
expect(bg.created[0]).toMatchObject({ left: 460, top: 60 });
|
||||
});
|
||||
|
||||
test("not centred on an approval window the user was last in", async () => {
|
||||
const bg = loadBackground({
|
||||
lastFocused: {
|
||||
type: "popup",
|
||||
left: 440,
|
||||
top: 0,
|
||||
width: 1280,
|
||||
height: 720,
|
||||
},
|
||||
});
|
||||
|
||||
bg.requestSign();
|
||||
await settle();
|
||||
|
||||
// Centred, it would be at left 900, the position the browser refused.
|
||||
expect(bg.created).toHaveLength(1);
|
||||
expect(bg.created[0].left).toBeUndefined();
|
||||
expect(bg.created[0].top).toBeUndefined();
|
||||
});
|
||||
|
||||
test("placed by the browser when it refuses the centred position", async () => {
|
||||
const bg = loadBackground({
|
||||
refusePosition: true,
|
||||
lastFocused: {
|
||||
type: "normal",
|
||||
left: 1500,
|
||||
top: 900,
|
||||
width: 400,
|
||||
height: 300,
|
||||
},
|
||||
});
|
||||
|
||||
const sign = bg.requestSign();
|
||||
await settle();
|
||||
|
||||
expect(bg.created).toHaveLength(2);
|
||||
expect(bg.created[0]).toMatchObject({ left: 1520, top: 750 });
|
||||
expect(bg.created[1].left).toBeUndefined();
|
||||
expect(bg.created[1].top).toBeUndefined();
|
||||
|
||||
// The request waits on the second window rather than failing:
|
||||
// closing that window is refusing the prompt.
|
||||
expect(sign.result()).toBeNull();
|
||||
bg.closeWindow(2);
|
||||
await settle();
|
||||
expect(sign.result()).toEqual({
|
||||
error: { code: 4001, message: "User rejected the request." },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+82
-1
@@ -2676,7 +2676,9 @@ function dappMessages(page, type) {
|
||||
|
||||
// The approval window the background opened. Approvals are raised from an
|
||||
// RPC call rather than from a user gesture, so the extension opens a real
|
||||
// popup window for them; it is an ordinary page in this context.
|
||||
// popup window for them; it is an ordinary page in this context. It is the
|
||||
// first one found: no test starts with an approval window open (see main()),
|
||||
// so within a test it can only be this test's.
|
||||
async function waitForApprovalWindow(ctx, timeout = 30000) {
|
||||
const deadline = Date.now() + timeout;
|
||||
for (;;) {
|
||||
@@ -3633,6 +3635,55 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
|
||||
);
|
||||
});
|
||||
|
||||
// The extension used to centre every approval window on the last focused
|
||||
// window. Centred on another approval window, the new one landed where the
|
||||
// browser refused to create it, and the request failed with no window at all
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/290).
|
||||
test("a prompt raised while another approval window has focus opens its own (#290)", async (env) => {
|
||||
await startRequest(env.dapp, "focus-sign", "personal_sign", [
|
||||
SIGN_HEX,
|
||||
env.expectedAddress,
|
||||
]);
|
||||
const signWindow = await waitForApprovalWindow(env.ctx);
|
||||
await visible(signWindow, "#view-approve-sign");
|
||||
await signWindow.bringToFront();
|
||||
const focused = await env.page.evaluate(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
chrome.windows.getLastFocused((w) => resolve(w.type));
|
||||
}),
|
||||
);
|
||||
assert(
|
||||
focused === "popup",
|
||||
"the sign window does not have focus, so this proves nothing: the " +
|
||||
"last focused window is a " +
|
||||
focused,
|
||||
);
|
||||
|
||||
const opened = env.ctx.waitForEvent("page");
|
||||
await startRequest(env.dapp, "focus-tx", "eth_sendTransaction", [
|
||||
{
|
||||
from: env.expectedAddress,
|
||||
to: STUB_COUNTERPARTY,
|
||||
value: toQuantity(TX_VALUE_WEI),
|
||||
data: TX_DATA,
|
||||
},
|
||||
]);
|
||||
const txWindow = await opened.catch(async () => {
|
||||
const outcome = await settleRequest(env.dapp, "focus-tx", 1000);
|
||||
throw new Error(
|
||||
"the extension opened no window for the transaction: " +
|
||||
JSON.stringify(outcome),
|
||||
);
|
||||
});
|
||||
await visible(txWindow, "#view-approve-tx");
|
||||
|
||||
// Closing a window is refusing its prompt, so both answer 4001.
|
||||
await closeApprovalPages(env.ctx);
|
||||
await assertUserRejection(env.dapp, "focus-tx", "the transaction prompt");
|
||||
await assertUserRejection(env.dapp, "focus-sign", "the sign prompt");
|
||||
});
|
||||
|
||||
// The closing pass over both boundaries at once. Every message the section
|
||||
// put on either channel is re-read here and required to be free of the
|
||||
// password — and required to be there at all, method by method, so the
|
||||
@@ -4002,6 +4053,36 @@ async function main() {
|
||||
failure = e.message;
|
||||
}
|
||||
|
||||
// No test starts with an approval window open. One that closes itself,
|
||||
// after a signature or on Reject, can still be closing when the next
|
||||
// test raises its prompt, and waitForApprovalWindow() would take it
|
||||
// for the new one. After a test that passed, its windows get five
|
||||
// seconds to close, and one still open then fails the test: it should
|
||||
// have closed itself, and closing it here unreported would hide that.
|
||||
// After a test that already failed, they are closed unreported, so
|
||||
// that a prompt it left unanswered does not refuse the next one from
|
||||
// its site.
|
||||
if (!failure) {
|
||||
const deadline = Date.now() + 5000;
|
||||
let open;
|
||||
for (;;) {
|
||||
open = env.ctx
|
||||
.pages()
|
||||
.filter(
|
||||
(p) => !p.isClosed() && p.url().includes("?approval="),
|
||||
);
|
||||
if (open.length === 0 || Date.now() > deadline) break;
|
||||
await sleep(50);
|
||||
}
|
||||
if (open.length > 0) {
|
||||
failure =
|
||||
"an approval window was still open 5000ms after the " +
|
||||
"test: " +
|
||||
open.map((p) => p.url()).join(", ");
|
||||
}
|
||||
}
|
||||
await closeApprovalPages(env.ctx);
|
||||
|
||||
// Any uncaught page error, console.error or unstubbed request
|
||||
// fails the test that provoked it, whether or not its assertions
|
||||
// passed. This is the mechanism that caught #150.
|
||||
|
||||
Reference in New Issue
Block a user