Compare commits

...
2 Commits
Author SHA1 Message Date
clawbot 61b8fbc58a fix: open no approval window for a site-connection prompt already answered (closes #287)
check / check (push) Failing after 3s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
When a site-connection prompt was decided before the toolbar popup raised
for it had loaded, that popup was torn down, chrome.action.openPopup()
rejected, and the background opened its fallback window for the answered
approval and only then removed it. In the Chrome end-to-end suite the next
test could take that window for its own prompt and lose it under its wait.
openApprovalWindow() now returns before creating a window when the approval
is no longer pending.

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

Model: opus-5-5
2026-10-05 01:44:25 +00:00
clawbot 8c8caafe33 harden: lost-password confirmation refuses empty input and ignores invisible characters (closes #336)
check / check (push) Failing after 3s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
A wallet named only with spaces compared equal to an empty field, so
typing nothing would have deleted it, and a zero-width space in a name
made the name impossible to type back.

An empty typed confirmation is now refused whatever the name is. The
characters src/shared/symbolSpoof.js already defines as painting nothing
are removed from both sides before comparing. A name that shows nothing
at all is shown on the delete screens as "Wallet N", so it can still be
typed back.

Model: opus-5-5
2026-10-05 03:26:05 +02:00
8 changed files with 175 additions and 34 deletions
+4 -5
View File
@@ -22,11 +22,10 @@ on: [push]
# These jobs REPORT, they do not gate. Whether a check blocks a merge is # 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 # 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 # 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 # block. Making e2e-chrome a required check is blocked while reports of the
# flake in the dApp signing wait -- two of six runs of unmutated code on a # Chrome suite failing under load are still open; the "In CI" section of
# loaded machine -- tracked as # README.md names them. A gate that fails at random teaches people to merge
# https://git.eeqj.de/sneak/AutistMask/issues/287. A gate that fails at # past red.
# random teaches people to merge past red.
# #
# Nothing here may pass vacuously. There is no continue-on-error and no # 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 # `|| true`. Both scripts exit non-zero when docker is missing, when the
+22 -13
View File
@@ -619,13 +619,14 @@ 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 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. 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 That is not only a statement about configuration. Reports of the Chrome suite
**measurably flaky under load** — two of six runs of unmutated code on a busy **failing under load** are still open, among them
machine lost the approval popup out from under the dApp signing wait, always in [#290](https://git.eeqj.de/sneak/AutistMask/issues/290), runs on a busy machine
the `#183` section, tracked as failing with `the extension opened no approval window within 30000ms`, and
[#287](https://git.eeqj.de/sneak/AutistMask/issues/287). So a red `e2e-chrome` [#446](https://git.eeqj.de/sneak/AutistMask/issues/446), the Settings round trip
has to be read before it is believed, and that flake is the blocker to ever closing the popup before its network switch is saved. So a red `e2e-chrome` has
making this a required check. Do not answer it with a retry wrapper: a suite 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 wrapper: a suite
that reruns until it is green stops being evidence. that reruns until it is green stops being evidence.
Nothing in either job can pass vacuously. There is no `continue-on-error` and no Nothing in either job can pass vacuously. There is no `continue-on-error` and no
@@ -1788,7 +1789,10 @@ view would leave a wallet one click from deletion.
new password — and, in bold, that without that phrase written down the new password — and, in bold, that without that phrase written down the
deletion loses everything the wallet holds, forever deletion loses everything the wallet holds, forever
- That the other wallets are not touched - That the other wallets are not touched
- The wallet's name, and a text input asking for it to be typed back - The wallet's name, and a text input asking for it to be typed back. A name
that shows nothing at all (only spaces, or only characters that paint
nothing) is shown as "Wallet N", its position in the list, and that is
what is typed back.
- Error line - Error line
- "Delete This Wallet Forever" button - "Delete This Wallet Forever" button
- **Transitions**: - **Transitions**:
@@ -1796,9 +1800,9 @@ view would leave a wallet one click from deletion.
outcomes as "Confirm Delete" above, through the same `finishDelete()`, so outcomes as "Confirm Delete" above, through the same `finishDelete()`, so
the selection repair, permission cleanup and `AUTISTMASK_ACTIVE_CHANGED` the selection repair, permission cleanup and `AUTISTMASK_ACTIVE_CHANGED`
broadcast are identical on both routes broadcast are identical on both routes
- "Delete This Wallet Forever" (name does not match) → "That is not the name - "Delete This Wallet Forever" (name does not match, or the field is empty)
of this wallet. Type <name> to confirm." on the error line, nothing → "That is not the name of this wallet. Type <name> to confirm." on
deleted the error line, nothing deleted
- "Back" → **DeleteWallet**, re-entered through its `show()` so the wallet - "Back" → **DeleteWallet**, re-entered through its `show()` so the wallet
selection comes back with it. The two delete screens are siblings rather selection comes back with it. The two delete screens are siblings rather
than parent and child: nothing is pushed on the way here, so both have than parent and child: nothing is pushed on the way here, so both have
@@ -1807,8 +1811,13 @@ view would leave a wallet one click from deletion.
secret protects nobody: an attacker at the popup who wants the wallet gone can secret protects nobody: an attacker at the popup who wants the wallet gone can
uninstall the extension, so the only person such a gate stops is the owner who uninstall the extension, so the only person such a gate stops is the owner who
forgot it. The typed name is a check that the user knows which wallet they are forgot it. The typed name is a check that the user knows which wallet they are
on, not a secret, so it is matched with surrounding spaces and letter case on, not a secret, so it is matched as the user can see it: letter case,
ignored. surrounding spaces and repeated inner spaces are ignored, and characters that
paint nothing (format characters such as the zero-width space,
default-ignorable characters, and DELETE — the same set
`src/shared/symbolSpoof.js` strips) are removed from both sides before
comparing. An empty field, or one holding only spaces or such characters, is
refused whatever the wallet is called.
- Not in `RESTORABLE_VIEWS`, alongside `delete-wallet-confirm`: a popup reopened - Not in `RESTORABLE_VIEWS`, alongside `delete-wallet-confirm`: a popup reopened
by accident must not land on a screen whose button erases key material. by accident must not land on a screen whose button erases key material.
+24
View File
@@ -45,6 +45,30 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-10-05: The extension no longer opens a window for a site-connection
prompt already answered
([#287](https://git.eeqj.de/sneak/AutistMask/issues/287)). When the prompt was
decided before the toolbar popup raised for it had loaded, that popup was torn
down, `chrome.action.openPopup()` rejected, and the background opened its
fallback window for the answered approval and then removed it. In the Chrome
end-to-end suite the next test could take that window for its own prompt and
lose it under its wait. `openApprovalWindow()` now opens nothing for an
approval that is no longer pending. The blocklist test's Reject, whose window
closes itself, is clicked as the other site Reject is, with the click
witnessed. Making `e2e-chrome` a required check is still blocked: other
reports of the Chrome suite failing under load are open, among them
[#290](https://git.eeqj.de/sneak/AutistMask/issues/290) and
[#446](https://git.eeqj.de/sneak/AutistMask/issues/446), as `README.md` says.
- 2026-10-05: The lost-password delete confirmation refuses an empty field and
ignores characters that paint nothing
([#336](https://git.eeqj.de/sneak/AutistMask/issues/336)). A wallet named only
with spaces compared equal to an empty field, so typing nothing would have
deleted it, and a zero-width space in a name made the name impossible to type
back. An empty field is now refused whatever the name is, the same invisible
characters `src/shared/symbolSpoof.js` strips are removed from both sides, and
a name that shows nothing is shown and typed back as "Wallet N".
- 2026-10-05: A `holders_count` that is not a whole number in plain digits is - 2026-10-05: A `holders_count` that is not a whole number in plain digits is
unknown, not read in part unknown, not read in part
([#251](https://git.eeqj.de/sneak/AutistMask/issues/251)). `parseInt` read ([#251](https://git.eeqj.de/sneak/AutistMask/issues/251)). `parseInt` read
+5 -1
View File
@@ -413,7 +413,7 @@ function releaseApproval(approval) {
} }
} }
// Open approval in a separate popup window. // Open approval in a separate popup window, unless it is no longer pending.
// This is the primary mechanism for tx/sign approvals (triggered programmatically, // This is the primary mechanism for tx/sign approvals (triggered programmatically,
// not from a user gesture) and the fallback for site-connection approvals. // not from a user gesture) and the fallback for site-connection approvals.
// Never rejects. Its callers raise it from inside a Promise executor and drop // Never rejects. Its callers raise it from inside a Promise executor and drop
@@ -446,6 +446,10 @@ async function openApprovalWindow(id) {
); );
} }
// Already answered: a site-connection prompt decided before the toolbar
// popup raised for it had loaded, whose openPopup() rejects only now.
if (!pendingApprovals[id]) return;
let win = null; let win = null;
try { try {
win = await windowsCreate(opts); win = await windowsCreate(opts);
+26 -13
View File
@@ -12,6 +12,7 @@ const {
removeWalletFromState, removeWalletFromState,
broadcastActiveChanged, broadcastActiveChanged,
} = require("../../shared/walletDelete"); } = require("../../shared/walletDelete");
const { INVISIBLE_CHARACTERS } = require("../../shared/symbolSpoof");
let deleteWalletIndex = null; let deleteWalletIndex = null;
let lostPasswordIndex = null; let lostPasswordIndex = null;
@@ -20,21 +21,31 @@ let ctx = null;
// The name shown for a wallet, and on the lost-password screen the string // The name shown for a wallet, and on the lost-password screen the string
// the user has to type back. One function so the two cannot disagree: a // the user has to type back. One function so the two cannot disagree: a
// confirmation that asks for a name other than the one on screen is // confirmation that asks for a name other than the one on screen is
// unusable. // unusable. A name that shows nothing at all (only spaces, or only
// zero-width characters) is replaced by "Wallet N" for the same reason:
// there would be nothing on screen to type back.
function displayName(walletIdx) { function displayName(walletIdx) {
const wallet = state.wallets[walletIdx]; const wallet = state.wallets[walletIdx];
return (wallet && wallet.name) || "Wallet " + (walletIdx + 1); const name = wallet && wallet.name;
if (name && confirmKey(name)) return name;
return "Wallet " + (walletIdx + 1);
} }
// What the typed confirmation and the wallet name are compared as. HTML // What the typed confirmation and the wallet name are compared as. HTML
// collapses runs of whitespace when it renders the name, so a wallet named // collapses runs of whitespace when it renders the name, so a wallet named
// "My Wallet" with two spaces DISPLAYS as "My Wallet": the user cannot // "My Wallet" with two spaces DISPLAYS as "My Wallet": the user cannot
// see the second space and cannot type a string that matches the stored // see the second space and cannot type a string that matches the stored
// name. Comparing collapsed on both sides is what keeps the confirmation // name. Characters that paint nothing, such as a zero-width space, are
// satisfiable, on the one screen whose whole purpose is unwedging a user // invisible the same way and are removed first. Comparing this form on
// who is already stuck. Case and surrounding space go the same way. // both sides is what keeps the confirmation satisfiable, on the one screen
// whose whole purpose is unwedging a user who is already stuck. Case and
// surrounding space go the same way.
function confirmKey(name) { function confirmKey(name) {
return name.trim().replace(/\s+/g, " ").toLowerCase(); return name
.replace(INVISIBLE_CHARACTERS, "")
.trim()
.replace(/\s+/g, " ")
.toLowerCase();
} }
// Drop the password from the DOM and the wallet selection from the // Drop the password from the DOM and the wallet selection from the
@@ -174,14 +185,16 @@ function init(_ctx) {
return; return;
} }
// Case, surrounding spaces and repeated inner spaces are not part // Case, surrounding spaces, repeated inner spaces and invisible
// of the confirmation; see confirmKey(). This asks whether the // characters are not part of the confirmation; see confirmKey().
// user knows which wallet they are on; it is not a secret, and // This asks whether the user knows which wallet they are on; it is
// refusing "wallet 2" for "Wallet 2" would only teach the user to // not a secret, and refusing "wallet 2" for "Wallet 2" would only
// distrust the control. // teach the user to distrust the control. An empty field is
const typed = $("delete-wallet-lost-name-input").value; // refused whatever the wallet is called, so no stored name can
// ever be confirmed by typing nothing.
const typed = confirmKey($("delete-wallet-lost-name-input").value);
const expected = displayName(lostPasswordIndex); const expected = displayName(lostPasswordIndex);
if (confirmKey(typed) !== confirmKey(expected)) { if (typed === "" || typed !== confirmKey(expected)) {
$("delete-wallet-lost-flash").textContent = $("delete-wallet-lost-flash").textContent =
"That is not the name of this wallet. Type " + "That is not the name of this wallet. Type " +
expected + expected +
+21
View File
@@ -2235,6 +2235,27 @@ describe("a site connection decided as the popup closes", () => {
}); });
}); });
// The prompt is decided before the toolbar popup raised for it has
// loaded; that popup is torn down and openPopup() rejects only after.
test("a toolbar prompt already decided opens no window when openPopup() rejects", async () => {
const bg = loadBackground({ actionPopup: true });
const opening = deferred();
bg.openPopup.mockImplementation(() => opening.promise);
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
opening.reject(new Error("the toolbar popup closed before it loaded"));
await settle();
expect(bg.created).toHaveLength(0);
});
// The port carries a decision now, so it carries the sender check the // 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 // 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. // approval id must not be able to connect the site it is running on.
+69
View File
@@ -46,6 +46,9 @@ const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599"; const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
// U+200B, built from its code point so that it can be seen in this file.
const ZERO_WIDTH_SPACE = String.fromCodePoint(0x200b);
// ------------------------------------------------------------ DOM stub // ------------------------------------------------------------ DOM stub
function makeElement(id) { function makeElement(id) {
@@ -342,6 +345,72 @@ describe("the typed confirmation", () => {
"secret-three", "secret-three",
]); ]);
}); });
// A name of only spaces compares as nothing, and so does an empty
// field. Typing nothing must still delete nothing.
test.each(["", " "])(
"typing %j deletes nothing when the name is only spaces",
async (typedValue) => {
const { deleteWallet, state, storage } = load();
state.wallets[1].name = " ";
await openLostPassword(deleteWallet, 1);
node("delete-wallet-lost-name-input").value = typedValue;
await click("btn-delete-wallet-lost-confirm");
expect(node("delete-wallet-lost-flash").style.visibility).toBe(
"visible",
);
expect(state.wallets).toHaveLength(3);
expect(await persistedWallets(storage)).toHaveLength(3);
},
);
// A name that shows nothing would leave nothing on screen to type
// back, so the screen names the wallet by its position instead, and
// that is what the user types.
test.each([
["spaces", " "],
["a zero-width space", ZERO_WIDTH_SPACE],
])(
"a name of only %s is shown and typed back as Wallet 2",
async (_label, storedName) => {
const { deleteWallet, state, storage } = load();
state.wallets[1].name = storedName;
await openLostPassword(deleteWallet, 1);
expect(node("delete-wallet-lost-name").textContent).toBe(
"Wallet 2",
);
node("delete-wallet-lost-name-input").value = "Wallet 2";
await click("btn-delete-wallet-lost-confirm");
const persisted = await persistedWallets(storage);
expect(persisted.map((w) => w.encryptedSecret)).toEqual([
"secret-one",
"secret-three",
]);
},
);
// A zero-width space paints nothing, so "My", a zero-width space and
// "Wallet" reads as "MyWallet", and that is all the user can type. HTML
// does not collapse it the way it collapses spaces, so it has to be
// removed explicitly.
test("a zero-width space inside the name is not part of it", async () => {
const { deleteWallet, state, storage } = load();
state.wallets[1].name = "My" + ZERO_WIDTH_SPACE + "Wallet";
await openLostPassword(deleteWallet, 1);
node("delete-wallet-lost-name-input").value = "MyWallet";
await click("btn-delete-wallet-lost-confirm");
const persisted = await persistedWallets(storage);
expect(persisted.map((w) => w.encryptedSecret)).toEqual([
"secret-one",
"secret-three",
]);
});
}); });
describe("deleting without the password", () => { describe("deleting without the password", () => {
+4 -2
View File
@@ -2738,7 +2738,7 @@ async function closeApprovalPages(ctx) {
// #btn-reject on the site prompt — NOT self-proving. A page that went away // #btn-reject on the site prompt — NOT self-proving. A page that went away
// without the click landing disconnects the approval port, the background // without the click landing disconnects the approval port, the background
// settles that as 4001, and 4001 is exactly what assertUserRejection // settles that as 4001, and 4001 is exactly what assertUserRejection
// accepts. That call site arms the click trace below and asserts it. // accepts. Both call sites arm the click trace below and assert it.
// //
// A button that is missing or unclickable raises a different error, which is // A button that is missing or unclickable raises a different error, which is
// rethrown. // rethrown.
@@ -3124,7 +3124,9 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
// Not remembered: a remembered decision for this origin would // Not remembered: a remembered decision for this origin would
// outlive the test. // outlive the test.
await popup.uncheck("#approve-remember"); await popup.uncheck("#approve-remember");
await popup.click("#btn-reject"); await armClickTrace(env, popup, "#btn-reject");
await clickAndClose(popup, "#btn-reject");
await assertClickLanded(env, "#btn-reject");
await assertUserRejection( await assertUserRejection(
phishingDapp, phishingDapp,