Compare commits

..

1 Commits

Author SHA1 Message Date
722f7c86de feat: vendor and censor the phishing blocklist at build time (closes #219)
All checks were successful
check / check (push) Successful in 27s
e2e / e2e-chrome (push) Successful in 48s
e2e / e2e-firefox (push) Successful in 40s
The blocklist URL in shipped code named a competitor and pointed at a moving
ref, and the extension re-fetched from it every 24 hours, which also meant a
third party decided what this wallet warns about. All of that is gone.

script/vendor-blocklist fetches upstream at a pinned commit, verifies the
sha256 of the bytes that commit serves, and writes
src/shared/phishingBlocklist.json. It is build-time tooling, never shipped, and
the one place in the repo that names the upstream project; a source reference
nobody can verify is not a source reference.

The artifact stores truncated sha256 digests rather than domain names. That is
what censors it: the previous file contained the competitor's name 6,475 times,
as phishing domains impersonating them, and not one of those domains is
dropped. It also makes lookups a binary search over a fixed-width string, so
nothing is built at module load — which matters on MV3, where the worker
re-evaluates the module on every wake — and takes the file from 8.7 MB to
1.7 MB.

script/check-censored enforces the rest: it reads the name out of the vendoring
script rather than repeating it, and fails on any occurrence in the working
tree or under dist/ that is not one of the two literals shipped code cannot
avoid. It runs in make check, which inspects dist/ when there is one and says
loudly when there is not, and again with --require-dist at the end of every
make build.

Removing the runtime fetch retires the delta, the extension-storage persistence
and the 24-hour alarm from #158. A retired alarm is now cleared rather than
left waking the worker forever on installs that already have it.

The e2e suite drives the warning end to end from a real blocklisted origin
served as a real http(s) site, with a control asserting the banner stays hidden
for one that is not listed. Its service-worker interception canary needed a new
anchor, since the startup fetch it used to watch for no longer happens: it now
wakes the worker with a message and asks it for one throwaway fetch.

LICENSE no longer cites a repository that returns 404.

eslint.config.js gains one block: script/lib/ holds node programs the shell
entrypoints call, and without it they lint with no globals at all.
2026-08-17 07:18:09 +00:00
8 changed files with 121 additions and 596 deletions

View File

@@ -103,12 +103,12 @@ provide:
fmt-check
- `script/check-censored` — assert the competitor name RULES.md bars appears
nowhere in the working tree or under `dist/` outside its documented
exceptions: the pinned source reference in `script/vendor-blocklist`, the two
provider-shim identifiers in `src/content/inpage.js`, and one ERC-20's
on-chain name in `src/shared/tokenList.js`. Each is scoped to that path and
fails anywhere else. Part of `make check`, which inspects `dist/` when there
is one and says loudly when there is not; `make build` re-runs it with
`--require-dist`, so a build artifact is always covered
exceptions, which are the pinned source reference in `script/vendor-blocklist`
and the two literals shipped code cannot avoid (the `isMetaMask`/`_metamask`
provider shim, and one ERC-20's on-chain name in the token list). Part of
`make check`, which inspects `dist/` when there is one and says loudly when
there is not; `make build` re-runs it with `--require-dist`, so a build
artifact is always covered
- `script/vendor-blocklist` — refresh `src/shared/phishingBlocklist.json` from
its upstream, pinned to a commit and to the sha256 of the bytes that commit
serves. Run deliberately, never as part of a build: the output is committed
@@ -1807,14 +1807,12 @@ The full license texts for these third-party files are included in the
[LICENSE](LICENSE) file. The `eth-phishing-detect` row carries no repository
link because the upstream is hosted under a competitor's organization name,
which project policy keeps out of code and documentation.
`script/vendor-blocklist` is the single definition and the only file that spells
the name in prose: it is build-time tooling, never shipped, and it records the
exact URL, the commit it is pinned to and the sha256 of the bytes that commit
serves, because a source reference nobody can verify is not a source reference.
`script/check-censored` reads the name back out of that one file and fails the
build wherever else it appears, save for two shipped-code literals it cannot
avoid — each permitted only at the one path that carries it, and listed in that
script's header.
`script/vendor-blocklist` is the single exception and the single definition: it
is build-time tooling, never shipped, and it records the exact URL, the commit
it is pinned to and the sha256 of the bytes that commit serves, because a source
reference nobody can verify is not a source reference. `script/check-censored`
reads the name back out of that one file and fails the build if it appears
anywhere else.
## Author

25
TODO.md
View File

@@ -81,13 +81,12 @@ but the review is broader than any of them.
domain names — which is what removes the competitor's name from a list that
carried it 6,475 times, without dropping a single one of those domains.
`script/check-censored` runs in `make check` and again against `dist/` at the
end of every build, each permitted occurrence scoped to the one path allowed
to carry it; the name now appears only in the vendoring script, which defines
it once, in the provider-shim identifiers in `src/content/inpage.js`, and in
one ERC-20's on-chain name in `src/shared/tokenList.js`. Removing the fetch
retired the delta, the persistence and the 24-hour alarm from
[#158](https://git.eeqj.de/sneak/AutistMask/issues/158), and retired alarms
are now cleared rather than left running on existing installs. Two
end of every build; the name now appears only in the vendoring script, which
defines it once, and in the two literals shipped code cannot avoid (the
`isMetaMask`/`_metamask` provider shim, and one ERC-20's on-chain name).
Removing the fetch retired the delta, the persistence and the 24-hour alarm
from [#158](https://git.eeqj.de/sneak/AutistMask/issues/158), and retired
alarms are now cleared rather than left running on existing installs. Two
consequences, both deliberate: the list no longer self-updates, so it is as
fresh as the last vendoring run that was released; and re-vendoring from
current upstream took it from 231,357 stale entries to 105,721 current ones,
@@ -249,18 +248,6 @@ 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

View File

@@ -9,27 +9,25 @@
# - script/vendor-blocklist. Build-time tooling, never shipped. A pinned
# source reference that does not say what the source is cannot be verified
# by anyone, so it names it. Whole-file exemption.
# - the two provider-shim identifiers in src/content/inpage.js. Protocol
# identifiers dApps feature-detect on; renaming them does not rename them in
# their code, it only stops this wallet working on their sites.
# - the on-chain name of the MUSD ERC-20 in src/shared/tokenList.js. It is not
# what backs symbol-spoof detection — that reads symbol and address — but
# the wallet already surfaces the on-chain name of any token the user holds
# (src/shared/balances.js), and this contract's on-chain name is that
# string, so censoring the repo cannot stop the wallet displaying it.
# Dropping the entry instead would cost the user MUSD spoof detection.
# - the literal `isMetaMask` and the `_metamask` object in
# src/content/inpage.js. A protocol identifier dApps feature-detect on;
# renaming it does not rename it in their code, it only stops this wallet
# working on their sites.
# - the literal `MetaMask USD` in src/shared/tokenList.js. The on-chain name
# of an ERC-20 the user may hold, recorded next to its address and symbol,
# which is what lets symbol-spoofing detection tell the real one from a
# forgery. Altering it would misidentify an asset to its owner.
#
# Everything else fails, in the working tree and under dist/. The last two are
# literals rather than whole files, so they are enforced by counting, and each
# literal is scoped to the path allowed to carry it: a file may contain the name
# only as many times as it contains the literals permitted *there*, and zero
# times anywhere else. The emitted bundles carry them too, so a plain "the name
# must not appear in dist/" could never have passed.
# literals rather than files, so they are enforced by counting: a file may
# contain the name only as many times as it contains those exact strings. The
# emitted bundles carry both, so a plain "the name must not appear in dist/"
# could never have passed.
#
# The name itself is not written in this file. script/vendor-blocklist is the
# one place in this repo that defines it, and this reads it back out of there —
# so the repo-wide grep this check exists to enforce keeps returning exactly the
# files named above, and this file is not one of them.
# The name itself is not written here. script/vendor-blocklist is the one place
# in this repo that defines it, and this reads it back out of there — so the
# repo-wide grep this check exists to enforce keeps returning exactly the files
# named above, and this file is not one of them.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -44,7 +42,7 @@ SCAN_FLAG="--scan-paths"
VENDOR_SCRIPT="$ROOT/script/vendor-blocklist"
# Set by extract_name / make_literals_file.
# Set by extract_name / write_allowed_literals.
NAME=""
ALLOWED_LITERALS_FILE=""
@@ -81,32 +79,17 @@ extract_name() {
esac
}
make_literals_file() {
# Occurrences allowed anywhere, including in the emitted bundles. Each contains
# the name exactly once, which is what makes counting them sound.
write_allowed_literals() {
ALLOWED_LITERALS_FILE="$(mktemp \
"${TMPDIR:-/tmp}/autistmask-censored.XXXXXX")" ||
fail "could not create a temporary file, so nothing was scanned."
}
# The literals $1 may carry, and nothing else may. Each contains the name
# exactly once, which is what makes counting them sound; each is scoped to its
# path, so a file with no business carrying the name fails even when it spells
# it the way shipped code has to. Scoping is the point: permitting these
# literals in any file is what once let this check pass its own prose.
#
# The emitted paths are listed next to the sources they come from. If the
# bundler moves one, this goes red and the new path gets added deliberately,
# rather than a wildcard over dist/ covering whatever lands there.
allowed_literals_for() {
: >"$ALLOWED_LITERALS_FILE"
case "$1" in
src/content/inpage.js | dist/*/src/content/inpage.js)
printf 'is%s\n_%s\n' "$NAME" "$NAME" >"$ALLOWED_LITERALS_FILE"
;;
src/shared/tokenList.js | dist/*/src/background/index.js | \
dist/*/src/popup/index.js)
printf '%s USD\n' "$NAME" >"$ALLOWED_LITERALS_FILE"
;;
esac
{
echo "is$NAME"
echo "_$NAME"
echo "$NAME USD"
} >"$ALLOWED_LITERALS_FILE"
}
# How many times does $1 contain the name (TOTAL), and how many of those are one
@@ -131,14 +114,6 @@ count_matches() {
return 0
fi
# No literal is permitted at this path, so every occurrence is a violation.
# Handled here rather than by grep, which is not required to say anything
# useful about an empty pattern file.
if [ ! -s "$ALLOWED_LITERALS_FILE" ]; then
ALLOWED=0
return 0
fi
_cm_status=0
_cm_out="$(grep -a -o -i -F -f "$ALLOWED_LITERALS_FILE" -- "$1")" ||
_cm_status=$?
@@ -156,26 +131,17 @@ count_matches() {
# counting as everything else rather than a second copy of it.
scan_paths() {
for _file in "$@"; do
# dist/ arrives absolute (find) and the worktree relative (git
# ls-files). The allowlist is keyed on repo-relative paths, so both
# forms are reduced to one before anything is decided about them.
_rel="$_file"
case "$_rel" in
"$ROOT"/*) _rel="${_rel#"$ROOT"/}" ;;
esac
case "$_rel" in
script/vendor-blocklist) continue ;;
case "$_file" in
"$VENDOR_SCRIPT" | "script/vendor-blocklist") continue ;;
esac
[ -f "$_file" ] || continue
allowed_literals_for "$_rel"
count_matches "$_file"
[ "$TOTAL" -gt "$ALLOWED" ] || continue
FAILED=$((FAILED + 1))
echo "check-censored: $_rel: $TOTAL occurrence(s) of the name," \
"$ALLOWED of them allowed at this path" >&2
echo "check-censored: $_file: $TOTAL occurrence(s) of the name," \
"$ALLOWED of them allowed" >&2
grep -a -n -i -F -e "$NAME" -- "$_file" | cut -c1-140 | head -5 >&2
done
[ "$FAILED" -eq 0 ]
@@ -244,7 +210,7 @@ main() {
if [ "${1-}" = "$SCAN_FLAG" ]; then
shift
extract_name
make_literals_file
write_allowed_literals
scan_paths "$@"
return $?
fi
@@ -257,7 +223,7 @@ main() {
esac
extract_name
make_literals_file
write_allowed_literals
echo "Checking for censored names..."

View File

@@ -489,53 +489,13 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
});
}
// 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.
// 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.
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) {
@@ -545,6 +505,7 @@ runtime.onConnect.addListener((port) => {
}
settleApproval(id, { approved: false, remember: false });
}
resetPopupUrl();
});
}
});
@@ -1099,21 +1060,10 @@ 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,
@@ -1163,16 +1113,18 @@ 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) && !isExtensionSender(sender)) {
sendResponse({ error: "Unauthorized sender" });
return false;
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 (msg.type === "AUTISTMASK_GET_APPROVAL") {
@@ -1204,6 +1156,15 @@ 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;

View File

@@ -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;
approvalPort = runtimeApi().connect({ name: "approval:" + id });
runtimeApi().connect({ name: "approval:" + id });
let details = null;
try {
@@ -476,14 +476,6 @@ 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
@@ -551,28 +543,6 @@ 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);
@@ -583,11 +553,25 @@ function init(_ctx) {
});
$("btn-approve").addEventListener("click", () => {
decideSite(true);
const remember = $("approve-remember").checked;
notify({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: true,
remember,
});
window.close();
});
$("btn-reject").addEventListener("click", () => {
decideSite(false);
const remember = $("approve-remember").checked;
notify({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: false,
remember,
});
window.close();
});
$("btn-approve-tx").addEventListener("click", async () => {

View File

@@ -39,21 +39,6 @@ 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 = {
@@ -185,13 +170,8 @@ 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: {
@@ -210,14 +190,7 @@ function loadBackground(options) {
messageListener = fn;
},
},
// 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;
},
},
onConnect: { addListener: () => {} },
lastError: null,
},
windows: {
@@ -245,17 +218,7 @@ function loadBackground(options) {
query: (q, cb) => cb([]),
sendMessage: () => {},
},
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() } : {}),
},
action: { setPopup: () => {} },
};
require("../src/background/index");
@@ -323,70 +286,6 @@ 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) {
@@ -397,8 +296,6 @@ function loadBackground(options) {
send,
requestTx,
requestSign,
requestSite,
connectApproval,
closeWindow,
broadcastTransaction,
loadState,
@@ -1718,197 +1615,3 @@ 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." },
});
});
});

View File

@@ -2184,13 +2184,32 @@ async function reserveApprovalTab(env) {
// one down with it.
env.approvalTab = await env.ctx.newPage();
// 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).
// 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 () {};
});
await env.approvalTab.goto("about:blank");
await sleep(APPROVAL_TAB_SETTLE_MS);
return env.approvalTab;
@@ -2239,95 +2258,6 @@ 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
@@ -2557,11 +2487,7 @@ 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");
// 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 popup.click("#btn-reject");
await assertUserRejection(
env.dapp,
@@ -2592,7 +2518,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 clickAndClose(popup, "#btn-approve");
await popup.click("#btn-approve");
outcome = await settleRequest(env.dapp, "accounts");
} finally {
@@ -2747,7 +2673,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 clickAndClose(popup, "#btn-reject-sign");
await popup.click("#btn-reject-sign");
await assertUserRejection(
env.dapp,
@@ -2850,7 +2776,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 clickAndClose(popup, "#btn-reject-sign");
await popup.click("#btn-reject-sign");
await assertUserRejection(
env.dapp,
@@ -3008,7 +2934,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
]);
const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-tx");
await clickAndClose(popup, "#btn-reject-tx");
await popup.click("#btn-reject-tx");
await assertUserRejection(
env.dapp,

View File

@@ -384,7 +384,7 @@ describe("the shipped token list", () => {
"0xab5eb14c09d416f0ac63661e57edb7aecdb9befa", // Metronome Synth USD
],
MUSD: [
"0xaca92e438df0b2401ff60da7e4337b687a2435da",
"0xaca92e438df0b2401ff60da7e4337b687a2435da", // MetaMask USD
"0xdd468a1ddc392dcdbef6db6e34e89aa338f9f186", // Mezo USD
],
JPYC: [