Compare commits

..

1 Commits

Author SHA1 Message Date
d32ffe7c3a fix: settle a site approval on the port that carries its teardown (closes #275)
All checks were successful
check / check (push) Successful in 30s
Approve and window.close() left the popup on the next line, and the decision
and the disconnect the close caused travelled independent channels with nothing
ordering them. The disconnect handler settled a pending site approval as a
rejection, so whichever landed first decided the outcome. Driven in a tab the
teardown won every time: the user allowed the connection and the dApp was told
they had refused.

The decision now goes out on the approval port the popup already opens, which
is the same port the close disconnects. One channel is ordered -- a message
posted on a port is delivered before that port's own disconnect -- so the
approval is settled before the teardown is even seen, and the disconnect then
finds nothing pending to reject. Nothing waits, nothing is timed, and the popup
closes exactly as immediately as before.

windows.onRemoved no longer decides a site approval whose port is connected
either. In the fallback-window shape that event races the decision on a channel
of its own, which is the same defect one level over; the port disconnect says
the same thing in a defined order, so it is left to say it. A window that
closes before its popup ever connected has nothing else to speak for it and is
still rejected there, so no dApp is left waiting on a window that is gone.

Rejecting reports a rejection, and so does closing without deciding, in both
shapes. AUTISTMASK_APPROVAL_RESPONSE is gone; the port name carries the
approval id, so the popup no longer names one, and the sender check the message
carried moved to the port.

tests/backgroundApproval.test.js drives decide-then-disconnect with nothing
awaited in between, in the toolbar-popup shape that production uses and in the
fallback-window shape, and asserts every close-without-deciding path still
rejects. tests/e2e/run.js drops the deferred-window.close() accommodation it
carried for this bug, so the two site-prompt tests now drive the shipped
decide-then-close in a real Chromium.
2026-08-14 04:22:16 +00:00
11 changed files with 427 additions and 325 deletions

View File

@@ -1,49 +0,0 @@
name: e2e
on: [push]
# The browser end-to-end suites, one job per browser, deliberately kept out
# of the check workflow: REPO_POLICIES.md caps make test at 20 seconds and
# script/cibuild is a plain `docker build .` whose Dockerfile runs
# make check, so folding a browser suite into either would blow that cap
# and slow the local fast path. Before this workflow every browser-level
# guarantee in this repo held only when a human remembered to run it.
#
# One job per browser rather than two steps in one job, so a Chrome failure
# does not hide the Firefox result.
#
# Each job is one script and nothing else. Both scripts need docker and
# nothing else — they deliver the repo to the daemon as a build context and
# build the extension inside the pinned image — which is what makes them
# runnable here at all: the runner executes the job in a container against
# the host's docker socket, so a `-v "$PWD:/work"` source path is resolved
# by the host daemon and mounts an empty directory, and the runner image's
# node is too old to install this repo's dependencies.
#
# These jobs REPORT, they do not gate. Whether a check blocks a merge is
# Gitea branch protection, which this repo does not configure, so a failure
# here is a red mark a reviewer has to account for rather than a hard
# block. Making e2e-chrome a required check is blocked on the measured
# flake in the dApp signing wait -- two of six runs of unmutated code on a
# loaded machine -- tracked as
# https://git.eeqj.de/sneak/AutistMask/issues/287. A gate that fails at
# random teaches people to merge past red.
#
# Nothing here may pass vacuously. There is no continue-on-error and no
# `|| true`. Both scripts exit non-zero when docker is missing, when the
# image build fails, and when the browser fails to start; the Chrome
# harness aborts the suite outright if its network interception is not in
# effect.
jobs:
e2e-chrome:
runs-on: ubuntu-latest
steps:
# actions/checkout v4.2.2, 2026-02-22
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
- run: script/test-e2e
e2e-firefox:
runs-on: ubuntu-latest
steps:
# actions/checkout v4.2.2, 2026-02-22
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
- run: script/test-e2e-firefox

View File

@@ -83,11 +83,10 @@ provide:
git pre-commit hook git pre-commit hook
- `script/projectname` — print the project name (used for the Docker image tag) - `script/projectname` — print the project name (used for the Docker image tag)
- `script/test` — run the test suite (jest) - `script/test` — run the test suite (jest)
- `script/test-e2e` — run the Chrome browser end-to-end suite (docker is the - `script/test-e2e` — run the Chrome browser end-to-end suite (docker required;
only prerequisite: it builds a pinned image that carries the repo and a fresh see [End-to-End Tests](#end-to-end-tests))
extension build, see [End-to-End Tests](#end-to-end-tests)) - `script/test-e2e-firefox` — run the Firefox browser end-to-end suite (docker
- `script/test-e2e-firefox` — run the Firefox browser end-to-end suite (same, required; builds its own pinned image, see
against an image with a pinned Firefox and geckodriver, see
[End-to-End Tests](#end-to-end-tests)) [End-to-End Tests](#end-to-end-tests))
- `script/lint` — run the linter - `script/lint` — run the linter
- `script/fmt` — format all files (writes) - `script/fmt` — format all files (writes)
@@ -137,12 +136,11 @@ are outside `make check`.
`make test-e2e` builds `dist/chrome/` and drives the **real popup in a real `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
Chrome**, loaded as an unpacked MV3 extension inside a pinned Chrome**, loaded as an unpacked MV3 extension inside a pinned
`mcr.microsoft.com/playwright` container (pinned by digest in `mcr.microsoft.com/playwright` container (pinned by digest in `script/test-e2e`;
`tests/e2e/Dockerfile`, which is also where the extension is built; docker is docker is required and the suite fails loudly rather than skipping if it is
required and the suite fails loudly rather than skipping if it is unavailable). unavailable). The suite lives in `tests/e2e/` and is driven by
The suite lives in `tests/e2e/` and is driven by `playwright-core`, whose `playwright-core`, whose version must stay matched to the container's Playwright
version must stay matched to the container's Playwright version — the browsers version — the browsers ship inside the image.
ship inside the image.
It covers popup load, WebAssembly compilation under the shipped CSP (see It covers popup load, WebAssembly compilation under the shipped CSP (see
[Content Security Policy](#content-security-policy)), wallet creation through [Content Security Policy](#content-security-policy)), wallet creation through
@@ -322,46 +320,9 @@ Two limits are worth knowing, both real differences from the Chrome suite:
Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or
`make test`. `REPO_POLICIES.md` caps `make test` at 20 seconds and a browser `make test`. `REPO_POLICIES.md` caps `make test` at 20 seconds and a browser
suite does not fit; nothing in `tests/e2e/` is named `*.test.js`, so jest cannot suite does not fit; nothing in `tests/e2e/` is named `*.test.js`, so jest cannot
pick it up either. Run them locally before changing anything under pick it up either. Neither is wired into the Gitea workflow yet —
`src/popup/views/`. docker-in-docker in CI is a separate question. Run them locally before changing
anything under `src/popup/views/`.
### In CI
`.gitea/workflows/e2e.yml` runs both suites on every push, as two jobs —
`e2e-chrome` and `e2e-firefox` — separate from the `check` workflow, so the
20-second `make test` cap and the local fast path are untouched. Each job is a
checkout and the matching `script/` entrypoint, nothing else.
Docker is the only thing either job needs from the runner, and that is not an
accident. The runner executes a job inside a container against the **host's**
docker daemon, so a `docker run -v "$PWD:/work"` source path is resolved by the
host and mounts an empty directory, and the runner image's node is too old to
install this repo's dependencies. Both suites therefore ship the repo to the
daemon as a build context and build the extension inside the image, which works
identically on a laptop.
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. The Chrome suite is
**measurably flaky under load** — two of six runs of unmutated code on a busy
machine lost the approval popup out from under the dApp signing wait, always in
the `#183` section, tracked as
[#287](https://git.eeqj.de/sneak/AutistMask/issues/287). So a red `e2e-chrome`
has to be read before it is believed, and that flake is the blocker to ever
making this a required check. Do not answer it 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
`|| true`; both scripts exit non-zero when docker is missing, when the image
build fails, and when the browser fails to start; the Chrome harness aborts the
suite outright if its network interception is not in effect.
Measured on this repo's runner: `e2e-chrome` about 1m55s cold, almost all of it
the one-time pull of the pinned ~800MB Playwright layer, and well under a minute
once that layer is cached. `e2e-firefox` about 1m05s cold, and it caches its
Firefox and geckodriver downloads the same way.
## Rationale ## Rationale

37
TODO.md
View File

@@ -33,8 +33,7 @@ The backlog lives on the
authoritative; this file does not duplicate it. Full policy file set present. authoritative; this file does not duplicate it. Full policy file set present.
Real-browser end-to-end suites (`make test-e2e` for Chrome, Real-browser end-to-end suites (`make test-e2e` for Chrome,
`make test-e2e-firefox` for Firefox) now sit alongside `make check`, which `make test-e2e-firefox` for Firefox) now sit alongside `make check`, which
cannot see a runtime `ReferenceError` in a popup view, and cannot see a runtime `ReferenceError` in a popup view.
`.gitea/workflows/e2e.yml` runs both of them on every push.
# Next Step # Next Step
@@ -46,24 +45,18 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-14: CI runs the browser end-to-end suites. `.gitea/workflows/e2e.yml` - 2026-08-14: Approving a site connection is no longer a race against the popup
runs `script/test-e2e` and `script/test-e2e-firefox` as two jobs on every closing. The decision now rides the approval port the popup already holds,
push, separate from `check`, so `make check` and its 20-second `make test` cap which is the same channel the close disconnects, so it is delivered ahead of
are untouched. Every browser-level guarantee in this repo — the WASM-under-CSP that disconnect however fast the teardown is; `windows.onRemoved` no longer
check, the recovery-phrase and private-key DOM wipes, the ConfirmTx spend decides a site approval whose port is connected, since that event is ordered
gate, the dApp approval round trips — was enforced only when a human against nothing either. Rejecting and closing without deciding both still
remembered to run it by hand. The suites could not run on the runner as they report a rejection, and the popup delays its own close by nothing. The e2e
stood: the runner executes a job in a container against the host's docker harness's deferred-`window.close()` accommodation is gone with it, so the two
daemon, so `docker run -v "$PWD:/work"` mounts an empty directory (measured), site-prompt tests now drive the shipped decide-then-close in a real Chromium;
and the runner image's node cannot install this repo's dependencies. Both against the unfixed code the approval came back to the page as
suites now ship the repo to the daemon as a build context and build the `{"settled":"rejected","code":4001}`
extension inside the pinned image, so docker is the only prerequisite on a ([#275](https://git.eeqj.de/sneak/AutistMask/issues/275)).
runner or a laptop, and both run the image by ID rather than by tag so
concurrent clones cannot swap it. The jobs report rather than gate — this repo
configures no branch protection, and the Chrome suite is measurably flaky
under load, filed as [#287](https://git.eeqj.de/sneak/AutistMask/issues/287)
rather than papered over
([#259](https://git.eeqj.de/sneak/AutistMask/issues/259)).
- 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js` - 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 rebuilt every failure as `new Error(error.message)`, so the code the
background produced and the content script relayed intact was dropped in the background produced and the content script relayed intact was dropped in the
@@ -387,5 +380,9 @@ tracker.
- Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC - Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC
input validation) before any 1.0rc tag. Individual filed issues are parts of input validation) before any 1.0rc tag. Individual filed issues are parts of
it, but the review is broader than any of them. it, but the review is broader than any of them.
- Decide whether docker-in-docker makes `make test-e2e` and
`make test-e2e-firefox` runnable in the Gitea workflow. Extending the Chrome
suite itself is tracked as
[#183](https://git.eeqj.de/sneak/AutistMask/issues/183).
- Cut 1.0.0 once the milestone is empty, then continue tagging as milestones - Cut 1.0.0 once the milestone is empty, then continue tagging as milestones
land. land.

View File

@@ -7,29 +7,17 @@
# caps make test at 20 seconds and a browser suite does not fit. Run it # caps make test at 20 seconds and a browser suite does not fit. Run it
# yourself before touching popup views; it is the only check that can see # yourself before touching popup views; it is the only check that can see
# a used-but-not-imported identifier blow up at runtime. # a used-but-not-imported identifier blow up at runtime.
# .gitea/workflows/e2e.yml also runs it on every push, in a job separate
# from check so that cap and the local fast path both stay intact.
#
# Docker is the only prerequisite. The repo reaches the container as a
# build context and the extension is built inside it (see
# tests/e2e/Dockerfile), so nothing here depends on the node, yarn or make
# on the machine that starts the run. That is not a convenience: a bind
# mount cannot work under Gitea Actions, and the runner image's node is too
# old to install this repo's dependencies.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome" # mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09
#
IIDFILE="" # The playwright-core devDependency is pinned to the matching Playwright
# version (1.56.0) and the two must be bumped together: the browsers ship
cleanup() { # inside this image, and playwright-core looks for the exact browser
if [ -n "$IIDFILE" ]; then # revision its own version expects. A mismatch fails at launch.
rm -f "$IIDFILE" IMAGE="mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2"
fi
}
main() { main() {
cd "$ROOT" cd "$ROOT"
@@ -39,23 +27,14 @@ main() {
exit 1 exit 1
fi fi
IIDFILE="$(mktemp)" echo "Building extension for e2e..."
trap cleanup EXIT yarn run build 2>&1
trap 'cleanup; exit 130' INT TERM
echo "Building the Chrome e2e image (extension included)..."
docker build --iidfile "$IIDFILE" -t "$IMAGE" -f tests/e2e/Dockerfile .
echo "Running e2e suite in the pinned Playwright container..." 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 # --ipc=host: Chromium's shared-memory needs more than the default
# 64MB /dev/shm or renderers crash. # 64MB /dev/shm or renderers crash.
# HOME=/tmp: the image's root home is not a reliable place for the # --user: keep files the suite touches owned by the caller, not root.
# browser profile. # HOME=/tmp: the mapped uid has no home directory in the image.
# PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1: without it, # PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1: without it,
# ctx.route() intercepts page requests only, and every fetch made by # ctx.route() intercepts page requests only, and every fetch made by
# the MV3 background service worker — including the phishing # the MV3 background service worker — including the phishing
@@ -72,10 +51,13 @@ main() {
# on a deliberate bump. # on a deliberate bump.
docker run --rm \ docker run --rm \
--ipc=host \ --ipc=host \
--user "$(id -u):$(id -g)" \
-e HOME=/tmp \ -e HOME=/tmp \
-e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \ -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
-e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \ -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
"$(cat "$IIDFILE")" \ -v "$ROOT:/work" \
-w /work \
"$IMAGE" \
node tests/e2e/run.js node tests/e2e/run.js
} }

View File

@@ -5,17 +5,12 @@
# #
# Deliberately NOT called by script/check or script/test, for the same # Deliberately NOT called by script/check or script/test, for the same
# reason as the Chrome suite: REPO_POLICIES.md caps make test at 20 seconds # reason as the Chrome suite: REPO_POLICIES.md caps make test at 20 seconds
# and a browser suite does not fit. .gitea/workflows/e2e.yml also runs it # and a browser suite does not fit.
# on every push, in a job separate from check.
# #
# Unlike script/test-e2e this builds its base image locally, because no # Unlike script/test-e2e this builds its image locally, because no
# published image carries both a pinned Firefox and a matching geckodriver. # published image carries both a pinned Firefox and a matching geckodriver.
# All three external artifacts are pinned by digest inside the Dockerfile; # All three external artifacts are pinned by digest inside the Dockerfile;
# see tests/e2e/firefox/Dockerfile, which also explains why the repo and # see tests/e2e/firefox/Dockerfile.
# the extension build are baked into the image rather than mounted.
#
# Docker is the only prerequisite: nothing here depends on the node, yarn
# or make on the machine that starts the run.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -23,14 +18,6 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox" IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
IIDFILE=""
cleanup() {
if [ -n "$IIDFILE" ]; then
rm -f "$IIDFILE"
fi
}
main() { main() {
cd "$ROOT" cd "$ROOT"
@@ -39,20 +26,16 @@ main() {
exit 1 exit 1
fi fi
IIDFILE="$(mktemp)" echo "Building extension for e2e..."
trap cleanup EXIT yarn run build 2>&1
trap 'cleanup; exit 130' INT TERM
echo "Building the pinned Firefox e2e image (extension included)..." # The build context is tests/e2e/firefox/ and holds nothing but the
docker build --iidfile "$IIDFILE" -t "$IMAGE" \ # Dockerfile: the harness itself arrives over the bind mount below, so
-f tests/e2e/firefox/Dockerfile . # editing it never invalidates an image layer.
echo "Building the pinned Firefox e2e image..."
docker build -t "$IMAGE" "$ROOT/tests/e2e/firefox"
echo "Running the Firefox e2e suite..." 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. # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
# --network none: the suite stubs nothing, so this is what keeps the # --network none: the suite stubs nothing, so this is what keeps the
# run offline and deterministic. The extension swallows its own # run offline and deterministic. The extension swallows its own
@@ -60,8 +43,8 @@ main() {
# network note in README.md. Weaker than the Chrome suite's # network note in README.md. Weaker than the Chrome suite's
# fixture interception, and honestly so — it proves no request # fixture interception, and honestly so — it proves no request
# escaped, but it cannot report which ones were attempted. # escaped, but it cannot report which ones were attempted.
# HOME=/tmp: the image's root home is not a reliable place for the # --user: keep files the suite touches owned by the caller, not root.
# browser profile. # HOME=/tmp: the mapped uid has no home directory in the image.
# #
# No --privileged. Firefox's sandbox logs # No --privileged. Firefox's sandbox logs
# "CanCreateUserNamespace() clone() failure: EPERM" on startup here; # "CanCreateUserNamespace() clone() failure: EPERM" on startup here;
@@ -69,8 +52,11 @@ main() {
docker run --rm \ docker run --rm \
--shm-size=1g \ --shm-size=1g \
--network none \ --network none \
--user "$(id -u):$(id -g)" \
-e HOME=/tmp \ -e HOME=/tmp \
"$(cat "$IIDFILE")" \ -v "$ROOT:/work" \
-w /work \
"$IMAGE" \
node tests/e2e/firefox/run.js dist/firefox node tests/e2e/firefox/run.js dist/firefox
} }

View File

@@ -279,13 +279,50 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
}); });
} }
// Detect when an approval popup (browser-action) closes without a response. // Anything only the extension's own pages may say. A content script speaks
// TX and sign approvals now use windows.create() and are handled by the // with the page's URL, so this is what separates the popup from the site the
// windowsApi.onRemoved listener below, but we still handle site-connection // popup is being asked about.
// approval disconnects here. 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
// windowsApi.onRemoved listener below.
runtime.onConnect.addListener((port) => { runtime.onConnect.addListener((port) => {
if (port.name.startsWith("approval:")) { if (port.name.startsWith("approval:")) {
const id = port.name.split(":")[1]; const id = port.name.split(":")[1];
if (pendingApprovals[id]) {
// This approval has a popup that can speak for it, so its
// disconnect is a trustworthy "closed"; see onRemoved below.
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,
});
resetPopupUrl();
});
port.onDisconnect.addListener(() => { port.onDisconnect.addListener(() => {
const approval = pendingApprovals[id]; const approval = pendingApprovals[id];
if (approval) { if (approval) {
@@ -832,20 +869,32 @@ startBackgroundJobs();
// window is an ordinary event with an attempt already in flight behind it. // window is an ordinary event with an attempt already in flight behind it.
// settleApproval() refuses those, which leaves the attempt to report its real // settleApproval() refuses those, which leaves the attempt to report its real
// outcome to the page. // outcome to the page.
//
// 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 (windowsApi && windowsApi.onRemoved) { if (windowsApi && windowsApi.onRemoved) {
windowsApi.onRemoved.addListener((windowId) => { windowsApi.onRemoved.addListener((windowId) => {
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.windowId !== windowId) continue; if (approval.windowId !== windowId) continue;
const rejection = const isSite = approval.type !== "tx" && approval.type !== "sign";
approval.type === "tx" || approval.type === "sign" if (isSite && approval.portConnected) continue;
? { settleApproval(
id,
isSite
? { approved: false, remember: false }
: {
error: { error: {
code: 4001, code: 4001,
message: "User rejected the request.", message: "User rejected the request.",
}, },
} },
: { approved: false, remember: false }; );
settleApproval(id, rejection);
} }
}); });
} }
@@ -872,18 +921,16 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
} }
// Validate that popup-only messages originate from the extension itself. // 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 = [ const POPUP_ONLY_TYPES = [
"AUTISTMASK_GET_APPROVAL", "AUTISTMASK_GET_APPROVAL",
"AUTISTMASK_APPROVAL_RESPONSE",
"AUTISTMASK_TX_RESPONSE", "AUTISTMASK_TX_RESPONSE",
"AUTISTMASK_SIGN_RESPONSE", "AUTISTMASK_SIGN_RESPONSE",
]; ];
if (POPUP_ONLY_TYPES.includes(msg.type)) { if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
const extUrl = runtime.getURL(""); sendResponse({ error: "Unauthorized sender" });
if (!sender.url || !sender.url.startsWith(extUrl)) { return false;
sendResponse({ error: "Unauthorized sender" });
return false;
}
} }
if (msg.type === "AUTISTMASK_GET_APPROVAL") { if (msg.type === "AUTISTMASK_GET_APPROVAL") {
@@ -915,15 +962,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
return false; 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") { if (msg.type === "AUTISTMASK_TX_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;

View File

@@ -441,7 +441,7 @@ function showSignApproval(details) {
function show(id) { function show(id) {
approvalId = id; approvalId = id;
runtime.connect({ name: "approval:" + id }); approvalPort = runtime.connect({ name: "approval:" + id });
runtime.sendMessage({ type: "AUTISTMASK_GET_APPROVAL", id }, (details) => { runtime.sendMessage({ type: "AUTISTMASK_GET_APPROVAL", id }, (details) => {
if (!details) { if (!details) {
window.close(); window.close();
@@ -470,6 +470,14 @@ function show(id) {
} }
let approvalId = null; 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; let pendingTxDetails = null;
// The exact objects shown to the user, kept so the popup signs what it // 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 // displayed rather than re-fetching or re-populating anything at approval
@@ -537,6 +545,20 @@ function clearSignPassword() {
hideError("approve-sign-error"); 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.
function decideSite(approved) {
if (approvalPort) {
approvalPort.postMessage({
type: "AUTISTMASK_APPROVAL_DECISION",
approved,
remember: $("approve-remember").checked,
});
}
window.close();
}
function init(ctx) { function init(ctx) {
onViewLeave("approve-tx", clearTxPassword); onViewLeave("approve-tx", clearTxPassword);
onViewLeave("approve-sign", clearSignPassword); onViewLeave("approve-sign", clearSignPassword);
@@ -547,25 +569,11 @@ function init(ctx) {
}); });
$("btn-approve").addEventListener("click", () => { $("btn-approve").addEventListener("click", () => {
const remember = $("approve-remember").checked; decideSite(true);
runtime.sendMessage({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: true,
remember,
});
window.close();
}); });
$("btn-reject").addEventListener("click", () => { $("btn-reject").addEventListener("click", () => {
const remember = $("approve-remember").checked; decideSite(false);
runtime.sendMessage({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: false,
remember,
});
window.close();
}); });
$("btn-approve-tx").addEventListener("click", async () => { $("btn-approve-tx").addEventListener("click", async () => {

View File

@@ -32,6 +32,21 @@ const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example"; const HOSTNAME = "dapp.example";
const EXT_URL = "chrome-extension://autistmask/"; 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 // What the dApp asks for: no nonce, no gas, no fees. This is the shape that
// makes a duplicate broadcast possible at all. // makes a duplicate broadcast possible at all.
const TX_PARAMS = { const TX_PARAMS = {
@@ -146,8 +161,13 @@ function loadBackground(options) {
let messageListener = null; let messageListener = null;
let windowRemovedListener = null; let windowRemovedListener = null;
let connectListener = null;
const created = []; const created = [];
const removed = []; 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 = { global.chrome = {
storage: { storage: {
@@ -163,7 +183,14 @@ function loadBackground(options) {
messageListener = fn; 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, lastError: null,
}, },
windows: { windows: {
@@ -189,7 +216,17 @@ function loadBackground(options) {
query: (q, cb) => cb([]), query: (q, cb) => cb([]),
sendMessage: () => {}, 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"); require("../src/background/index");
@@ -248,6 +285,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 // 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. // ids the window stub hands back, so window 1 is the first popup opened.
function closeWindow(windowId) { function closeWindow(windowId) {
@@ -258,6 +359,8 @@ function loadBackground(options) {
send, send,
requestTx, requestTx,
requestSign, requestSign,
requestSite,
connectApproval,
closeWindow, closeWindow,
broadcastTransaction, broadcastTransaction,
loadState, loadState,
@@ -1058,3 +1161,136 @@ 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.
test("approving in the fallback window survives the window event too", async () => {
const bg = loadBackground();
const pending = bg.requestSite();
await settle();
expect(bg.created).toHaveLength(1);
const port = bg.connectApproval(pending.id());
port.decide(true, false);
bg.closeWindow(1);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
});
// 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." },
});
});
});

View File

@@ -1,34 +0,0 @@
# Chrome end-to-end image: the pinned Playwright image with this repo and a
# freshly built extension inside it, built by script/test-e2e. The suite is
# still started with `docker run`, so every runtime flag the harness needs
# (--ipc=host in particular) applies as before.
#
# The repo is baked in rather than bind-mounted because a bind mount does
# not resolve under Gitea Actions: the runner runs the job in a container
# against the HOST's docker socket, so the source side of a -v is resolved
# by the host daemon while the job's checkout lives on a docker volume that
# is not a host path -- the mount silently succeeds and /work is empty. A
# build context is streamed to the daemon and so works from anywhere.
# Building the extension here too means the machine starting a run needs
# docker and nothing else.
# mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09
#
# The playwright-core devDependency is pinned to the matching Playwright
# version (1.56.0) and the two must be bumped together: the browsers ship
# inside this image, and playwright-core looks for the exact browser
# revision its own version expects. A mismatch fails at launch.
FROM mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2
WORKDIR /work
# Same layering as the root Dockerfile: script/bootstrap installs the
# prerequisites and the dependencies, and the manifests are copied first so
# that layer is cached until they change.
COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .
RUN make build

View File

@@ -1,24 +1,10 @@
# Firefox end-to-end image: stock Firefox plus geckodriver on a node base, # Firefox end-to-end image: stock Firefox plus geckodriver on a node base,
# with this repo and a freshly built extension inside it, built by # built by script/test-e2e-firefox. The repo is bind-mounted at /work; the
# script/test-e2e-firefox. The harness itself has no dependencies, so # harness itself has no dependencies, so nothing is installed for it.
# nothing is installed for it.
# #
# The build context is the repo root. The repo is baked in rather than # All three external artifacts are pinned by digest. The Firefox version in
# bind-mounted because a bind mount does not resolve under Gitea Actions: # particular must not float: -remote-allow-system-access is mandatory on 153
# the runner runs the job in a container against the HOST's docker socket, # and was not on 142, so the flag the harness passes is version-coupled.
# so the source side of a -v is resolved by the host daemon while the job's
# checkout lives on a docker volume that is not a host path -- the mount
# silently succeeds and /work is empty. Baking the build in is also the
# only way this suite can have both a built extension and the
# `--network none` it runs under, since a container with no network cannot
# install anything.
#
# All three external artifacts are pinned by digest, and are fetched in
# layers above the repo copy, so editing the harness or any source file
# re-runs only the two cheap layers at the bottom. The Firefox version in
# particular must not float: -remote-allow-system-access is mandatory on
# 153 and was not on 142, so the flag the harness passes is
# version-coupled.
# node:22-bookworm-slim, 2026-08-12 # node:22-bookworm-slim, 2026-08-12
FROM node@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436 FROM node@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436
@@ -62,16 +48,4 @@ ENV FIREFOX_BIN=/opt/firefox/firefox
ENV GECKODRIVER=/usr/local/bin/geckodriver ENV GECKODRIVER=/usr/local/bin/geckodriver
WORKDIR /work WORKDIR /work
# Same layering as the root Dockerfile: script/bootstrap installs the
# prerequisites and the dependencies, and the manifests are copied first so
# that layer is cached until they change.
COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .
RUN make build
CMD ["node", "tests/e2e/firefox/run.js", "dist/firefox"] CMD ["node", "tests/e2e/firefox/run.js", "dist/firefox"]

View File

@@ -1596,32 +1596,13 @@ async function reserveApprovalTab(env) {
// one down with it. // one down with it.
env.approvalTab = await env.ctx.newPage(); env.approvalTab = await env.ctx.newPage();
// The one accommodation this section makes to the shipped code, and the // This tab runs the shipped popup with nothing patched. The site
// reason for it. // approval buttons decide and then close on the next line, and the two
// // site-approval tests below are therefore the real-browser
// Both approval buttons call runtime.sendMessage() and then window.close() // approve-then-immediate-close and reject-then-immediate-close cases: the
// on the next line. Closing this page disconnects the approval port, and // decision rides the approval port, which also carries the disconnect the
// the disconnect handler in src/background/index.js settles a pending // close causes, so it is delivered ahead of it and the outcome does not
// site approval as a rejection. In a tab those two race and the teardown // depend on the teardown timing (#275).
// 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 env.approvalTab.goto("about:blank");
await sleep(APPROVAL_TAB_SETTLE_MS); await sleep(APPROVAL_TAB_SETTLE_MS);
return env.approvalTab; return env.approvalTab;
@@ -1670,6 +1651,28 @@ 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.
//
// This swallows nothing that matters: a click that did not land leaves the
// dApp promise unsettled and the assertion after the call still fails. 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;
}
}
// Record every message the approval window sends to the background worker. // Record every message the approval window sends to the background worker.
// //
// This is the direct observation the password check needs. It is installed // This is the direct observation the password check needs. It is installed
@@ -1890,7 +1893,7 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as
// origin in deniedSites and every later test in this section is // origin in deniedSites and every later test in this section is
// auto-rejected with no prompt at all, which would look like a pass. // auto-rejected with no prompt at all, which would look like a pass.
await popup.uncheck("#approve-remember"); await popup.uncheck("#approve-remember");
await popup.click("#btn-reject"); await clickAndClose(popup, "#btn-reject");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -1921,7 +1924,7 @@ test("eth_requestAccounts approved returns the selected address (#183)", async (
// does not, and the sign and transaction tests below all require the // does not, and the sign and transaction tests below all require the
// origin to still be authorized. // origin to still be authorized.
await popup.check("#approve-remember"); await popup.check("#approve-remember");
await popup.click("#btn-approve"); await clickAndClose(popup, "#btn-approve");
outcome = await settleRequest(env.dapp, "accounts"); outcome = await settleRequest(env.dapp, "accounts");
} finally { } finally {
@@ -2029,7 +2032,7 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await popup.click("#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2132,7 +2135,7 @@ test("eth_signTypedData_v4 rejected returns a rejection to the page (#183)", asy
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await popup.click("#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2290,7 +2293,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-tx"); await visible(popup, "#view-approve-tx");
await popup.click("#btn-reject-tx"); await clickAndClose(popup, "#btn-reject-tx");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,