Compare commits
2
Commits
f2b17fbec6
...
d9a516e489
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d9a516e489 | ||
|
|
d0bbb3d9eb |
@@ -45,6 +45,28 @@ but the review is broader than any of them.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-05: Each control that leads to a signature or to the private key has a
|
||||||
|
test that it refuses a defective wallet before asking for a password
|
||||||
|
([#254](https://git.eeqj.de/sneak/AutistMask/issues/254)): Send on the main,
|
||||||
|
address and token screens, Export Private Key, and both approval screens, as
|
||||||
|
drawn and as clicked. Send on the confirmation screen refuses it too now,
|
||||||
|
because the popup reopens onto that screen from a saved view. The comments
|
||||||
|
that said the wallet's key cannot be derived now say that
|
||||||
|
`getSignerForAddress` refuses it, and the module comment in
|
||||||
|
`src/shared/walletDefects.js` names both earlier import paths.
|
||||||
|
|
||||||
|
- 2026-10-05: The Chrome end-to-end suite drives the private key export screen
|
||||||
|
as it drives the recovery phrase screen
|
||||||
|
([#253](https://git.eeqj.de/sneak/AutistMask/issues/253)): the correct
|
||||||
|
password shows the key, leaving by the settings gear empties the screen, and
|
||||||
|
leaving while the password is still being checked never puts the key on it.
|
||||||
|
The cases use the imported key wallet rather than the HD one. Leaving drops
|
||||||
|
the address the screen was showing, and an HD wallet's key cannot be derived
|
||||||
|
without it, so on an HD wallet a late decrypt fails by itself and would never
|
||||||
|
exercise the check that discards it. The screen cannot yet be opened twice in
|
||||||
|
one popup session ([#460](https://git.eeqj.de/sneak/AutistMask/issues/460)),
|
||||||
|
so the cases reopen the popup before the second open.
|
||||||
|
|
||||||
- 2026-10-05: The StateRecovery screen is driven in a real browser under the
|
- 2026-10-05: The StateRecovery screen is driven in a real browser under the
|
||||||
shipped CSP, in both end-to-end suites
|
shipped CSP, in both end-to-end suites
|
||||||
([#361](https://git.eeqj.de/sneak/AutistMask/issues/361)). A stored record
|
([#361](https://git.eeqj.de/sneak/AutistMask/issues/361)). A stored record
|
||||||
|
|||||||
@@ -33,8 +33,8 @@ const { walletDefect } = require("../../shared/walletDefects");
|
|||||||
|
|
||||||
// The defect of the wallet the selected address belongs to, or null. Both the
|
// The defect of the wallet the selected address belongs to, or null. Both the
|
||||||
// send and the private-key export path check it before asking for a password,
|
// send and the private-key export path check it before asking for a password,
|
||||||
// so a wallet that cannot derive its keys says so instead of failing after the
|
// so a wallet whose key getSignerForAddress refuses says so instead of failing
|
||||||
// user has typed one in.
|
// after the user has typed one in.
|
||||||
function selectedWalletDefect() {
|
function selectedWalletDefect() {
|
||||||
if (state.selectedWallet === null) return null;
|
if (state.selectedWallet === null) return null;
|
||||||
return walletDefect(state.wallets[state.selectedWallet]);
|
return walletDefect(state.wallets[state.selectedWallet]);
|
||||||
@@ -259,9 +259,10 @@ function init(_ctx) {
|
|||||||
$("btn-export-privkey").addEventListener("click", () => {
|
$("btn-export-privkey").addEventListener("click", () => {
|
||||||
moreDropdown.classList.add("hidden");
|
moreDropdown.classList.add("hidden");
|
||||||
moreBtn.classList.remove("bg-fg", "text-bg");
|
moreBtn.classList.remove("bg-fg", "text-bg");
|
||||||
// There is no private key to export for an address this wallet
|
// This address's private key can be derived from the stored key,
|
||||||
// cannot derive. Without this the export screen would take a
|
// but export goes through getSignerForAddress, which refuses a key
|
||||||
// password and then report it as wrong.
|
// that is not a master key. Without this the export screen would
|
||||||
|
// take a password and then report that refusal as a wrong password.
|
||||||
const defect = selectedWalletDefect();
|
const defect = selectedWalletDefect();
|
||||||
if (defect) {
|
if (defect) {
|
||||||
showFlash(defect.shortMessage);
|
showFlash(defect.shortMessage);
|
||||||
|
|||||||
@@ -822,9 +822,10 @@ function setSignButtonBusy(busy) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Say so on the approval screen itself, and disable the approve button, when
|
// Say so on the approval screen itself, and disable the approve button, when
|
||||||
// the address the approval was raised for belongs to a wallet whose keys
|
// the address the approval was raised for belongs to a wallet whose key
|
||||||
// cannot be derived. Without this the screen would take a password and fail
|
// getSignerForAddress refuses. Without this the screen would take a password
|
||||||
// after deriving it. Reject stays available; the wallet is not touched.
|
// and fail after deriving it. Reject stays available; the wallet is not
|
||||||
|
// touched.
|
||||||
// Returns true when it gated.
|
// Returns true when it gated.
|
||||||
function gateOnWalletDefect(errorId, buttonId, address) {
|
function gateOnWalletDefect(errorId, buttonId, address) {
|
||||||
const owner = findWalletFor(address);
|
const owner = findWalletFor(address);
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ const {
|
|||||||
} = require("./helpers");
|
} = require("./helpers");
|
||||||
const { state, currentNetwork } = require("../../shared/state");
|
const { state, currentNetwork } = require("../../shared/state");
|
||||||
const { getSignerForAddress } = require("../../shared/wallet");
|
const { getSignerForAddress } = require("../../shared/wallet");
|
||||||
|
const { walletDefect } = require("../../shared/walletDefects");
|
||||||
const { decryptWithPassword } = require("../../shared/vault");
|
const { decryptWithPassword } = require("../../shared/vault");
|
||||||
const { formatUsd, getPrice } = require("../../shared/prices");
|
const { formatUsd, getPrice } = require("../../shared/prices");
|
||||||
const { getProvider } = require("../../shared/balances");
|
const { getProvider } = require("../../shared/balances");
|
||||||
@@ -537,6 +538,15 @@ function init(_ctx) {
|
|||||||
onViewLeave("confirm-tx", clearPassword);
|
onViewLeave("confirm-tx", clearPassword);
|
||||||
|
|
||||||
$("btn-confirm-send").addEventListener("click", async () => {
|
$("btn-confirm-send").addEventListener("click", async () => {
|
||||||
|
const wallet = state.wallets[state.selectedWallet];
|
||||||
|
// Every Send button refuses a defective wallet before this screen,
|
||||||
|
// but the popup also reopens onto it from a saved view.
|
||||||
|
const defect = walletDefect(wallet);
|
||||||
|
if (defect) {
|
||||||
|
showError("confirm-tx-password-error", defect.shortMessage);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const password = $("confirm-tx-password").value;
|
const password = $("confirm-tx-password").value;
|
||||||
if (!password) {
|
if (!password) {
|
||||||
showError(
|
showError(
|
||||||
@@ -546,7 +556,6 @@ function init(_ctx) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const wallet = state.wallets[state.selectedWallet];
|
|
||||||
let decryptedSecret;
|
let decryptedSecret;
|
||||||
hideError("confirm-tx-password-error");
|
hideError("confirm-tx-password-error");
|
||||||
|
|
||||||
|
|||||||
@@ -13,11 +13,17 @@ const NON_MASTER_XPRV = "non-master-xprv";
|
|||||||
|
|
||||||
// An "xprv" wallet stores the neutered BIP-44 Ethereum node, four levels below
|
// An "xprv" wallet stores the neutered BIP-44 Ethereum node, four levels below
|
||||||
// the key that was imported: the current import path derives the absolute
|
// the key that was imported: the current import path derives the absolute
|
||||||
// m/44'/60'/0'/0 from a depth-0 key, and the pre-#210 path derived the same
|
// m/44'/60'/0'/0 from a depth-0 key, and the path before #210 (57959b7)
|
||||||
// four levels as a relative path beneath whatever depth it was given. A master
|
// derived the same four levels as a relative path beneath whatever depth it
|
||||||
// import therefore stores a depth-4 xpub and a depth-d import stores depth
|
// was given. A master import therefore stores a depth-4 xpub and a depth-d
|
||||||
// d + 4, which makes the stored xpub an exact read on the imported key's
|
// import stores depth d + 4, which makes the stored xpub an exact read on the
|
||||||
// depth — and it is readable without the password, unlike the key itself.
|
// imported key's depth — and it is readable without the password, unlike the
|
||||||
|
// key itself.
|
||||||
|
//
|
||||||
|
// The first import path (7a7f9c5) does not fit: it stored the imported key's
|
||||||
|
// own xpub with no derivation, so a wallet it wrote is judged wrongly here (a
|
||||||
|
// master import as defective, a depth-4 import as sound). 57959b7 replaced it
|
||||||
|
// in the same push, and no tag contains it.
|
||||||
const BIP44_ETH_XPUB_DEPTH = 4;
|
const BIP44_ETH_XPUB_DEPTH = 4;
|
||||||
|
|
||||||
const DEFECTS = {
|
const DEFECTS = {
|
||||||
|
|||||||
+162
-26
@@ -606,22 +606,26 @@ async function openSettings(page) {
|
|||||||
await visible(page, "#view-settings");
|
await visible(page, "#view-settings");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Everything the recovery phrase screen is holding, read straight out of
|
// Everything a screen that shows a secret is holding, read straight out of
|
||||||
// the DOM whether or not that screen is the one on top. Reading it while it
|
// the DOM whether or not that screen is the one on top. Reading it while it
|
||||||
// is hidden is the point: "cleared on leave" means the node is empty, not
|
// is hidden is the point: "cleared on leave" means the node is empty, not
|
||||||
// merely off-screen.
|
// merely off-screen. `view` is "show-phrase" or "export-privkey"; the two
|
||||||
async function phraseScreenState(page) {
|
// screens name their elements the same way.
|
||||||
return page.evaluate(() => ({
|
async function secretScreenState(page, view) {
|
||||||
value: document.getElementById("show-phrase-value").textContent,
|
return page.evaluate(
|
||||||
error: document.getElementById("show-phrase-flash").textContent,
|
(v) => ({
|
||||||
html: document.getElementById("view-show-phrase").innerHTML,
|
value: document.getElementById(v + "-value").textContent,
|
||||||
|
error: document.getElementById(v + "-flash").textContent,
|
||||||
|
html: document.getElementById("view-" + v).innerHTML,
|
||||||
resultHidden: document
|
resultHidden: document
|
||||||
.getElementById("show-phrase-result")
|
.getElementById(v + "-result")
|
||||||
.classList.contains("hidden"),
|
.classList.contains("hidden"),
|
||||||
viewHidden: document
|
viewHidden: document
|
||||||
.getElementById("view-show-phrase")
|
.getElementById("view-" + v)
|
||||||
.classList.contains("hidden"),
|
.classList.contains("hidden"),
|
||||||
}));
|
}),
|
||||||
|
view,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function openPhraseScreen(page) {
|
async function openPhraseScreen(page) {
|
||||||
@@ -636,12 +640,12 @@ async function revealPhrase(page) {
|
|||||||
await visible(page, "#show-phrase-result", 60000);
|
await visible(page, "#show-phrase-result", 60000);
|
||||||
}
|
}
|
||||||
|
|
||||||
function assertWiped(st, phrase, where) {
|
function assertWiped(st, secret, where) {
|
||||||
assert(st.value === "", "phrase still in the DOM " + where);
|
assert(st.value === "", "the secret is still in the DOM " + where);
|
||||||
assert(st.resultHidden, "result section still shown " + where);
|
assert(st.resultHidden, "result section still shown " + where);
|
||||||
assert(
|
assert(
|
||||||
!st.html.includes(phrase),
|
!st.html.includes(secret),
|
||||||
"the recovery phrase is still somewhere in the screen markup " + where,
|
"the secret is still somewhere in the screen markup " + where,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -663,7 +667,8 @@ test("only an HD wallet is offered the recovery phrase action (#161)", async (en
|
|||||||
// The other half of the gate, against the real UI: a wallet holding a bare
|
// The other half of the gate, against the real UI: a wallet holding a bare
|
||||||
// private key has no phrase to show, so no row of it may offer the action.
|
// private key has no phrase to show, so no row of it may offer the action.
|
||||||
// The key is generated here rather than committed — the repo holds no
|
// The key is generated here rather than committed — the repo holds no
|
||||||
// private keys, test ones included.
|
// private keys, test ones included. It is kept on env for the private key
|
||||||
|
// export tests (#253).
|
||||||
test("a key wallet is not offered the recovery phrase action (#161)", async (env) => {
|
test("a key wallet is not offered the recovery phrase action (#161)", async (env) => {
|
||||||
const { Wallet } = require("ethers");
|
const { Wallet } = require("ethers");
|
||||||
|
|
||||||
@@ -671,10 +676,8 @@ test("a key wallet is not offered the recovery phrase action (#161)", async (env
|
|||||||
await env.page.click("#btn-main-add-wallet");
|
await env.page.click("#btn-main-add-wallet");
|
||||||
await visible(env.page, "#view-add-wallet");
|
await visible(env.page, "#view-add-wallet");
|
||||||
await env.page.click("#tab-privkey");
|
await env.page.click("#tab-privkey");
|
||||||
await env.page.fill(
|
env.privateKey = Wallet.createRandom().privateKey;
|
||||||
"#import-private-key",
|
await env.page.fill("#import-private-key", env.privateKey);
|
||||||
Wallet.createRandom().privateKey,
|
|
||||||
);
|
|
||||||
await env.page.fill("#add-wallet-password", PASSWORD);
|
await env.page.fill("#add-wallet-password", PASSWORD);
|
||||||
await env.page.fill("#add-wallet-password-confirm", PASSWORD);
|
await env.page.fill("#add-wallet-password-confirm", PASSWORD);
|
||||||
await env.page.click("#btn-add-wallet-confirm");
|
await env.page.click("#btn-add-wallet-confirm");
|
||||||
@@ -696,7 +699,7 @@ test("a key wallet is not offered the recovery phrase action (#161)", async (env
|
|||||||
|
|
||||||
test("the recovery phrase screen holds nothing before the password (#161)", async (env) => {
|
test("the recovery phrase screen holds nothing before the password (#161)", async (env) => {
|
||||||
await openPhraseScreen(env.page);
|
await openPhraseScreen(env.page);
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assertWiped(st, env.phrase, "before any password was entered");
|
assertWiped(st, env.phrase, "before any password was entered");
|
||||||
const passwordShown = await env.page.isVisible(
|
const passwordShown = await env.page.isVisible(
|
||||||
"#show-phrase-password-section",
|
"#show-phrase-password-section",
|
||||||
@@ -714,7 +717,7 @@ test("a wrong password reveals nothing (#161)", async (env) => {
|
|||||||
{ timeout: 60000 },
|
{ timeout: 60000 },
|
||||||
);
|
);
|
||||||
|
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assertWiped(st, env.phrase, "after a wrong password");
|
assertWiped(st, env.phrase, "after a wrong password");
|
||||||
assert(
|
assert(
|
||||||
/^[A-Z].*\.$/.test(st.error.trim()),
|
/^[A-Z].*\.$/.test(st.error.trim()),
|
||||||
@@ -730,7 +733,7 @@ test("the correct password reveals the full phrase, and nothing logs it (#161)",
|
|||||||
try {
|
try {
|
||||||
await revealPhrase(env.page);
|
await revealPhrase(env.page);
|
||||||
|
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assert(
|
assert(
|
||||||
st.value === env.phrase,
|
st.value === env.phrase,
|
||||||
"the displayed phrase is not the wallet's phrase, verbatim",
|
"the displayed phrase is not the wallet's phrase, verbatim",
|
||||||
@@ -761,7 +764,7 @@ test("the correct password reveals the full phrase, and nothing logs it (#161)",
|
|||||||
test('"Back" wipes the revealed phrase (#161)', async (env) => {
|
test('"Back" wipes the revealed phrase (#161)', async (env) => {
|
||||||
await env.page.click("#btn-show-phrase-back");
|
await env.page.click("#btn-show-phrase-back");
|
||||||
await visible(env.page, "#view-settings");
|
await visible(env.page, "#view-settings");
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assert(st.viewHidden, "the recovery phrase screen is still on top");
|
assert(st.viewHidden, "the recovery phrase screen is still on top");
|
||||||
assertWiped(st, env.phrase, "after Back");
|
assertWiped(st, env.phrase, "after Back");
|
||||||
});
|
});
|
||||||
@@ -773,7 +776,7 @@ test("leaving by the settings gear wipes it too (#161)", async (env) => {
|
|||||||
await revealPhrase(env.page);
|
await revealPhrase(env.page);
|
||||||
await env.page.click("#btn-settings");
|
await env.page.click("#btn-settings");
|
||||||
await visible(env.page, "#view-settings");
|
await visible(env.page, "#view-settings");
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assertWiped(st, env.phrase, "after leaving via the settings gear");
|
assertWiped(st, env.phrase, "after leaving via the settings gear");
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -810,7 +813,7 @@ test("leaving while the decrypt is in flight reveals nothing (#161)", async (env
|
|||||||
);
|
);
|
||||||
await sleep(2000);
|
await sleep(2000);
|
||||||
|
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
// Printed on every run, pass or fail: "the phrase is not there" is
|
// Printed on every run, pass or fail: "the phrase is not there" is
|
||||||
// worth more as a measurement than as a silent assertion, and the
|
// worth more as a measurement than as a silent assertion, and the
|
||||||
// same line read from a build without the guard is what this test
|
// same line read from a build without the guard is what this test
|
||||||
@@ -847,11 +850,141 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
|
|||||||
env.page = await openPopup(env.ctx, env.popupUrl);
|
env.page = await openPopup(env.ctx, env.popupUrl);
|
||||||
await visible(env.page, "#view-main");
|
await visible(env.page, "#view-main");
|
||||||
|
|
||||||
const st = await phraseScreenState(env.page);
|
const st = await secretScreenState(env.page, "show-phrase");
|
||||||
assert(st.viewHidden, "the popup reopened onto the recovery phrase screen");
|
assert(st.viewHidden, "the popup reopened onto the recovery phrase screen");
|
||||||
assertWiped(st, env.phrase, "after reopening the popup");
|
assertWiped(st, env.phrase, "after reopening the popup");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ------------------------------------------ private key export (#253)
|
||||||
|
|
||||||
|
// The recovery phrase cases above, on the private key export screen. They run
|
||||||
|
// against the key wallet imported above, not the HD wallet: leaving the screen
|
||||||
|
// drops the address it was showing, and without one an HD wallet's key cannot
|
||||||
|
// be derived, so there a decrypt that finished late would fail on its own and
|
||||||
|
// the liveness check would go untested.
|
||||||
|
|
||||||
|
// From Home to the export screen of the key wallet's one address. The key
|
||||||
|
// wallet is the second wallet in the list.
|
||||||
|
async function openPrivkeyScreen(page) {
|
||||||
|
await visible(page, "#view-main");
|
||||||
|
await page.click('#wallet-list .btn-addr-info[data-wallet="1"]');
|
||||||
|
await visible(page, "#view-address");
|
||||||
|
await page.click("#btn-more-menu");
|
||||||
|
await page.click("#btn-export-privkey");
|
||||||
|
await visible(page, "#view-export-privkey");
|
||||||
|
}
|
||||||
|
|
||||||
|
async function revealPrivkey(page) {
|
||||||
|
await page.fill("#export-privkey-password", PASSWORD);
|
||||||
|
await page.click("#btn-export-privkey-confirm");
|
||||||
|
await visible(page, "#export-privkey-result", 60000);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Leave the export screen, or the Settings screen the gear left it for, for
|
||||||
|
// Home. The gear put the export screen on the Back stack, so from Settings the
|
||||||
|
// way home passes through it, already emptied
|
||||||
|
// (https://git.eeqj.de/sneak/AutistMask/issues/461).
|
||||||
|
async function leavePrivkeyScreen(page) {
|
||||||
|
if (await page.isVisible("#view-settings")) {
|
||||||
|
await page.click("#btn-settings-back");
|
||||||
|
await visible(page, "#view-export-privkey");
|
||||||
|
}
|
||||||
|
if (await page.isVisible("#view-export-privkey")) {
|
||||||
|
await page.click("#btn-export-privkey-back");
|
||||||
|
await visible(page, "#view-address");
|
||||||
|
}
|
||||||
|
if (await page.isVisible("#view-address")) {
|
||||||
|
await page.click("#btn-address-back");
|
||||||
|
}
|
||||||
|
await visible(page, "#view-main");
|
||||||
|
}
|
||||||
|
|
||||||
|
test("the correct password reveals the private key, and nothing logs it (#253)", async (env) => {
|
||||||
|
const console_ = [];
|
||||||
|
const listener = (msg) => console_.push(msg.text());
|
||||||
|
env.page.on("console", listener);
|
||||||
|
try {
|
||||||
|
await openPrivkeyScreen(env.page);
|
||||||
|
await revealPrivkey(env.page);
|
||||||
|
|
||||||
|
const st = await secretScreenState(env.page, "export-privkey");
|
||||||
|
assert(
|
||||||
|
st.value === env.privateKey,
|
||||||
|
"the displayed key is not the wallet's private key, verbatim",
|
||||||
|
);
|
||||||
|
const promptShown = await env.page.isVisible(
|
||||||
|
"#export-privkey-password-section",
|
||||||
|
);
|
||||||
|
assert(!promptShown, "the password prompt is still shown after unlock");
|
||||||
|
|
||||||
|
const title = await env.page.getAttribute(
|
||||||
|
"#export-privkey-value",
|
||||||
|
"title",
|
||||||
|
);
|
||||||
|
assert(title === "Click to copy", "the key is not click-to-copy");
|
||||||
|
|
||||||
|
const leaked = console_.filter((line) => line.includes(env.privateKey));
|
||||||
|
assert(
|
||||||
|
leaked.length === 0,
|
||||||
|
"the private key reached the console: " + JSON.stringify(leaked),
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
env.page.off("console", listener);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("leaving by the settings gear wipes the private key (#253)", async (env) => {
|
||||||
|
await visible(env.page, "#export-privkey-result");
|
||||||
|
await env.page.click("#btn-settings");
|
||||||
|
await visible(env.page, "#view-settings");
|
||||||
|
const st = await secretScreenState(env.page, "export-privkey");
|
||||||
|
assertWiped(st, env.privateKey, "after leaving via the settings gear");
|
||||||
|
});
|
||||||
|
|
||||||
|
// The same interleaving as the recovery phrase case above, and for the same
|
||||||
|
// reason: both clicks in one page task, so the leave and its wipe run while
|
||||||
|
// the decrypt is still awaited. Reveal stays disabled while the decrypt runs,
|
||||||
|
// so reading it after the gear click shows the leave really came mid-decrypt.
|
||||||
|
test("leaving while the decrypt is in flight reveals no private key (#253)", async (env) => {
|
||||||
|
try {
|
||||||
|
await leavePrivkeyScreen(env.page);
|
||||||
|
// The export screen cannot yet be opened twice in one popup session
|
||||||
|
// (https://git.eeqj.de/sneak/AutistMask/issues/460), so this second
|
||||||
|
// open gets a fresh one.
|
||||||
|
await reopenPopup(env, "main");
|
||||||
|
await openPrivkeyScreen(env.page);
|
||||||
|
await env.page.fill("#export-privkey-password", PASSWORD);
|
||||||
|
const inFlight = await env.page.evaluate(() => {
|
||||||
|
const reveal = document.getElementById(
|
||||||
|
"btn-export-privkey-confirm",
|
||||||
|
);
|
||||||
|
reveal.click();
|
||||||
|
document.getElementById("btn-settings").click();
|
||||||
|
return reveal.disabled;
|
||||||
|
});
|
||||||
|
assert(
|
||||||
|
inFlight,
|
||||||
|
"the decrypt was not running when the screen was left",
|
||||||
|
);
|
||||||
|
await visible(env.page, "#view-settings");
|
||||||
|
|
||||||
|
// Reveal is re-enabled in the same continuation that would have
|
||||||
|
// written the key, so once it is back the decrypt has finished.
|
||||||
|
await env.page.waitForFunction(
|
||||||
|
() =>
|
||||||
|
!document.getElementById("btn-export-privkey-confirm").disabled,
|
||||||
|
null,
|
||||||
|
{ timeout: 60000 },
|
||||||
|
);
|
||||||
|
|
||||||
|
const st = await secretScreenState(env.page, "export-privkey");
|
||||||
|
assert(st.viewHidden, "the private key screen is still on top");
|
||||||
|
assertWiped(st, env.privateKey, "after leaving mid-decrypt");
|
||||||
|
} finally {
|
||||||
|
await leavePrivkeyScreen(env.page);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// ------------------------------- Back after reopening the popup (#268)
|
// ------------------------------- Back after reopening the popup (#268)
|
||||||
|
|
||||||
// A reopened popup renders the wallet list and the view it restores onto,
|
// A reopened popup renders the wallet list and the view it restores onto,
|
||||||
@@ -4201,6 +4334,9 @@ async function main() {
|
|||||||
// The recovery phrase of the wallet created in test 2, so later
|
// The recovery phrase of the wallet created in test 2, so later
|
||||||
// tests can assert on the real secret rather than its shape.
|
// tests can assert on the real secret rather than its shape.
|
||||||
phrase: null,
|
phrase: null,
|
||||||
|
// The private key of the key wallet imported by the recovery phrase
|
||||||
|
// tests (#161), asserted on by the private key export tests (#253).
|
||||||
|
privateKey: null,
|
||||||
// What the Settings section (#229) actually observed. A guard test
|
// What the Settings section (#229) actually observed. A guard test
|
||||||
// at the end of that section demands the full set, so a skipped or
|
// at the end of that section demands the full set, so a skipped or
|
||||||
// silently shortened assertion reddens the run instead of shrinking
|
// silently shortened assertion reddens the run instead of shrinking
|
||||||
|
|||||||
+294
-2
@@ -4,8 +4,16 @@
|
|||||||
// already in storage: the import that created it ran before the refusal
|
// already in storage: the import that created it ran before the refusal
|
||||||
// existed. Such a wallet used to sign for the wrong tree and now throws on the
|
// existed. Such a wallet used to sign for the wrong tree and now throws on the
|
||||||
// send screen instead. These tests pin down that it is named and explained in
|
// send screen instead. These tests pin down that it is named and explained in
|
||||||
// the wallet list, that nothing on the way there throws, and that a wallet
|
// the wallet list, that every control leading to a signature or to the private
|
||||||
// imported from a real master key is untouched by any of it.
|
// key refuses it before asking for a password, that nothing on the way there
|
||||||
|
// throws, and that a wallet imported from a real master key is untouched by
|
||||||
|
// any of it.
|
||||||
|
|
||||||
|
// Mocked so that no password has to be hashed: the controls below are checked
|
||||||
|
// for whether they decrypt at all.
|
||||||
|
jest.mock("../src/shared/vault", () => ({
|
||||||
|
decryptWithPassword: jest.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
const { HDNodeWallet, Mnemonic } = require("ethers");
|
const { HDNodeWallet, Mnemonic } = require("ethers");
|
||||||
|
|
||||||
@@ -237,6 +245,290 @@ describe("the wallet list", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A minimal DOM for driving the popup views: any element exists on first
|
||||||
|
// lookup, and click() runs the listeners a view attached to it.
|
||||||
|
function makeElement(id) {
|
||||||
|
const classes = new Set();
|
||||||
|
const el = {
|
||||||
|
id,
|
||||||
|
textContent: "",
|
||||||
|
title: "",
|
||||||
|
value: "",
|
||||||
|
innerHTML: "",
|
||||||
|
disabled: false,
|
||||||
|
style: {},
|
||||||
|
dataset: {},
|
||||||
|
listeners: {},
|
||||||
|
classList: {
|
||||||
|
add: (...names) => names.forEach((n) => classes.add(n)),
|
||||||
|
remove: (...names) => names.forEach((n) => classes.delete(n)),
|
||||||
|
contains: (n) => classes.has(n),
|
||||||
|
toggle: (n, force) => {
|
||||||
|
const on = force === undefined ? !classes.has(n) : force;
|
||||||
|
if (on) classes.add(n);
|
||||||
|
else classes.delete(n);
|
||||||
|
return on;
|
||||||
|
},
|
||||||
|
},
|
||||||
|
addEventListener: (name, fn) => {
|
||||||
|
el.listeners[name] = el.listeners[name] || [];
|
||||||
|
el.listeners[name].push(fn);
|
||||||
|
},
|
||||||
|
querySelectorAll: () => [],
|
||||||
|
appendChild: () => {},
|
||||||
|
};
|
||||||
|
// Views reach for .parentElement to hide whole sections.
|
||||||
|
Object.defineProperty(el, "parentElement", {
|
||||||
|
get: () => node(id + "-parent"),
|
||||||
|
});
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeDocument() {
|
||||||
|
const els = new Map();
|
||||||
|
return {
|
||||||
|
getElementById(id) {
|
||||||
|
// The debug banner is created on demand by helpers.js; absent
|
||||||
|
// is the state a non-debug, non-testnet popup is in.
|
||||||
|
if (id === "debug-banner") return null;
|
||||||
|
if (!els.has(id)) els.set(id, makeElement(id));
|
||||||
|
return els.get(id);
|
||||||
|
},
|
||||||
|
createElement: () => makeElement("created"),
|
||||||
|
addEventListener: () => {},
|
||||||
|
body: { prepend: () => {} },
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function node(id) {
|
||||||
|
return globalThis.document.getElementById(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
function click(id) {
|
||||||
|
return Promise.all((node(id).listeners.click || []).map((fn) => fn()));
|
||||||
|
}
|
||||||
|
|
||||||
|
// getSignerForAddress refuses this wallet's key, but only once the password
|
||||||
|
// has been typed and spent, and the screens report that refusal as a wrong
|
||||||
|
// password or a failed send. So every control that leads to it refuses first.
|
||||||
|
describe("every way to a signature or the private key refuses a defective wallet first", () => {
|
||||||
|
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
|
||||||
|
|
||||||
|
let state;
|
||||||
|
let decryptWithPassword;
|
||||||
|
let home;
|
||||||
|
let addressDetail;
|
||||||
|
let addressToken;
|
||||||
|
let approval;
|
||||||
|
let confirmTx;
|
||||||
|
|
||||||
|
let address;
|
||||||
|
let shortMessage;
|
||||||
|
// What the background answers when the approval window asks which
|
||||||
|
// approval it was opened for, and every message the popup sent it.
|
||||||
|
let approvalDetails;
|
||||||
|
let sent;
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
state = require("../src/shared/state").state;
|
||||||
|
decryptWithPassword =
|
||||||
|
require("../src/shared/vault").decryptWithPassword;
|
||||||
|
home = require("../src/popup/views/home");
|
||||||
|
addressDetail = require("../src/popup/views/addressDetail");
|
||||||
|
addressToken = require("../src/popup/views/addressToken");
|
||||||
|
approval = require("../src/popup/views/approval");
|
||||||
|
confirmTx = require("../src/popup/views/confirmTx");
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
const broken = brokenXprvWallet();
|
||||||
|
// A balance, so that no Send button's zero-balance refusal can stand
|
||||||
|
// in for the defect check.
|
||||||
|
broken.addresses[0].balance = "1.0000";
|
||||||
|
broken.addresses[0].tokenBalances = [];
|
||||||
|
address = broken.addresses[0].address;
|
||||||
|
shortMessage = walletDefect(broken).shortMessage;
|
||||||
|
|
||||||
|
approvalDetails = null;
|
||||||
|
sent = [];
|
||||||
|
globalThis.document = makeDocument();
|
||||||
|
globalThis.window = { close: () => {} };
|
||||||
|
globalThis.chrome = {
|
||||||
|
storage: { local: { get: async () => ({}), set: async () => {} } },
|
||||||
|
runtime: {
|
||||||
|
connect: () => ({ postMessage: () => {} }),
|
||||||
|
sendMessage: (msg, reply) => {
|
||||||
|
sent.push(msg);
|
||||||
|
if (!reply) return;
|
||||||
|
reply(
|
||||||
|
msg.type === "AUTISTMASK_GET_APPROVAL"
|
||||||
|
? approvalDetails
|
||||||
|
: null,
|
||||||
|
);
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
// What the wallet's stored secret decrypts to: the account-level key
|
||||||
|
// it was imported from.
|
||||||
|
decryptWithPassword.mockReset();
|
||||||
|
decryptWithPassword.mockResolvedValue(accountXprv(VECTOR_PHRASE));
|
||||||
|
|
||||||
|
state.wallets = [broken];
|
||||||
|
state.activeAddress = address;
|
||||||
|
state.selectedWallet = 0;
|
||||||
|
state.selectedAddress = 0;
|
||||||
|
state.selectedToken = "ETH";
|
||||||
|
state.viewStack = [];
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
state.wallets = [];
|
||||||
|
state.activeAddress = null;
|
||||||
|
state.selectedWallet = null;
|
||||||
|
state.selectedAddress = null;
|
||||||
|
state.selectedToken = null;
|
||||||
|
});
|
||||||
|
|
||||||
|
test("Send on the main screen", async () => {
|
||||||
|
state.currentView = "main";
|
||||||
|
home.init({});
|
||||||
|
|
||||||
|
await click("btn-main-send");
|
||||||
|
|
||||||
|
expect(node("flash-msg").textContent).toBe(shortMessage);
|
||||||
|
expect(state.currentView).toBe("main");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("Send on the address screen", async () => {
|
||||||
|
state.currentView = "address";
|
||||||
|
addressDetail.init({});
|
||||||
|
|
||||||
|
await click("btn-send");
|
||||||
|
|
||||||
|
expect(node("flash-msg").textContent).toBe(shortMessage);
|
||||||
|
expect(state.currentView).toBe("address");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("Export Private Key on the address screen", async () => {
|
||||||
|
state.currentView = "address";
|
||||||
|
addressDetail.init({});
|
||||||
|
|
||||||
|
await click("btn-export-privkey");
|
||||||
|
|
||||||
|
expect(node("flash-msg").textContent).toBe(shortMessage);
|
||||||
|
expect(state.currentView).toBe("address");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("Send on a token's screen", async () => {
|
||||||
|
state.currentView = "address-token";
|
||||||
|
addressToken.init({});
|
||||||
|
|
||||||
|
await click("btn-address-token-send");
|
||||||
|
|
||||||
|
expect(node("flash-msg").textContent).toBe(shortMessage);
|
||||||
|
expect(state.currentView).toBe("address-token");
|
||||||
|
});
|
||||||
|
|
||||||
|
// The popup reopens onto this screen from a saved view, so the Send
|
||||||
|
// buttons above are not the only way onto it. The screen is not drawn,
|
||||||
|
// because drawing it starts a fee estimate against the network; with a
|
||||||
|
// decrypt that fails, a handler without the check stops at the password
|
||||||
|
// instead of going on to a transaction that was never set up.
|
||||||
|
test("Send on the confirmation screen", async () => {
|
||||||
|
decryptWithPassword.mockRejectedValue(new Error("wrong password"));
|
||||||
|
state.currentView = "confirm-tx";
|
||||||
|
confirmTx.init({});
|
||||||
|
node("confirm-tx-password").value = "any password";
|
||||||
|
|
||||||
|
await click("btn-confirm-send");
|
||||||
|
|
||||||
|
expect(decryptWithPassword).not.toHaveBeenCalled();
|
||||||
|
expect(node("confirm-tx-password-error").textContent).toBe(
|
||||||
|
shortMessage,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
async function openTxApproval() {
|
||||||
|
approvalDetails = {
|
||||||
|
type: "tx",
|
||||||
|
origin: "https://dapp.example",
|
||||||
|
isPhishingDomain: false,
|
||||||
|
approvedFrom: address,
|
||||||
|
approvedTx: {
|
||||||
|
from: address,
|
||||||
|
to: RECIPIENT,
|
||||||
|
value: "0x0",
|
||||||
|
data: "0x",
|
||||||
|
chainId: 1,
|
||||||
|
nonce: 0,
|
||||||
|
gasLimit: "21000",
|
||||||
|
maxFeePerGas: "1000000000",
|
||||||
|
},
|
||||||
|
};
|
||||||
|
approval.init({});
|
||||||
|
await approval.show(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function openSignApproval() {
|
||||||
|
approvalDetails = {
|
||||||
|
type: "sign",
|
||||||
|
origin: "https://dapp.example",
|
||||||
|
isPhishingDomain: false,
|
||||||
|
approvedFrom: address,
|
||||||
|
// "Hello", as the hex a page sends.
|
||||||
|
signParams: {
|
||||||
|
method: "personal_sign",
|
||||||
|
message: "0x48656c6c6f",
|
||||||
|
from: address,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
approval.init({});
|
||||||
|
await approval.show(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
test("the transaction approval screen says so and disables Approve", async () => {
|
||||||
|
await openTxApproval();
|
||||||
|
|
||||||
|
expect(node("approve-tx-error").textContent).toBe(shortMessage);
|
||||||
|
expect(node("btn-approve-tx").disabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The stub runs a disabled button's listener, which a browser would not:
|
||||||
|
// what is asked here is whether the handler refuses on its own.
|
||||||
|
test("Approve on the transaction approval screen does not decrypt", async () => {
|
||||||
|
await openTxApproval();
|
||||||
|
node("approve-tx-password").value = "any password";
|
||||||
|
|
||||||
|
await click("btn-approve-tx");
|
||||||
|
|
||||||
|
expect(decryptWithPassword).not.toHaveBeenCalled();
|
||||||
|
expect(sent.map((msg) => msg.type)).not.toContain(
|
||||||
|
"AUTISTMASK_TX_RESPONSE",
|
||||||
|
);
|
||||||
|
expect(node("approve-tx-error").textContent).toBe(shortMessage);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the signature approval screen says so and disables Approve", async () => {
|
||||||
|
await openSignApproval();
|
||||||
|
|
||||||
|
expect(node("approve-sign-error").textContent).toBe(shortMessage);
|
||||||
|
expect(node("btn-approve-sign").disabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("Approve on the signature approval screen does not decrypt", async () => {
|
||||||
|
await openSignApproval();
|
||||||
|
node("approve-sign-password").value = "any password";
|
||||||
|
|
||||||
|
await click("btn-approve-sign");
|
||||||
|
|
||||||
|
expect(decryptWithPassword).not.toHaveBeenCalled();
|
||||||
|
expect(sent.map((msg) => msg.type)).not.toContain(
|
||||||
|
"AUTISTMASK_SIGN_RESPONSE",
|
||||||
|
);
|
||||||
|
expect(node("approve-sign-error").textContent).toBe(shortMessage);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("no path throws an unhandled error for a defective wallet", () => {
|
describe("no path throws an unhandled error for a defective wallet", () => {
|
||||||
test("address derivation from the stored xpub still works", () => {
|
test("address derivation from the stored xpub still works", () => {
|
||||||
// The stored xpub is at a non-standard depth but is a valid extended
|
// The stored xpub is at a non-standard depth but is a valid extended
|
||||||
|
|||||||
Reference in New Issue
Block a user