test: cover every control that refuses a defective wallet (closes #254)
check / check (push) Failing after 3s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 1s

Each control that leads to a signature or to the private key now has a
test that it refuses a defective wallet before decrypting anything: 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 had no such check. The Send buttons
stand in front of it, but the popup reopens onto it from a saved view,
so it now refuses the same way.

The comments that said the wallet's key cannot be derived now say that
getSignerForAddress refuses it, and the walletDefects module comment
names both earlier import paths.

Model: opus-5-5
This commit was merged in pull request #464.
This commit is contained in:
2026-10-05 12:59:15 +02:00
parent d0bbb3d9eb
commit 5d26283cd0
6 changed files with 335 additions and 16 deletions
+6 -5
View File
@@ -33,8 +33,8 @@ const { walletDefect } = require("../../shared/walletDefects");
// 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,
// so a wallet that cannot derive its keys says so instead of failing after the
// user has typed one in.
// so a wallet whose key getSignerForAddress refuses says so instead of failing
// after the user has typed one in.
function selectedWalletDefect() {
if (state.selectedWallet === null) return null;
return walletDefect(state.wallets[state.selectedWallet]);
@@ -259,9 +259,10 @@ function init(_ctx) {
$("btn-export-privkey").addEventListener("click", () => {
moreDropdown.classList.add("hidden");
moreBtn.classList.remove("bg-fg", "text-bg");
// There is no private key to export for an address this wallet
// cannot derive. Without this the export screen would take a
// password and then report it as wrong.
// This address's private key can be derived from the stored key,
// but export goes through getSignerForAddress, which refuses a key
// 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();
if (defect) {
showFlash(defect.shortMessage);
+4 -3
View File
@@ -822,9 +822,10 @@ function setSignButtonBusy(busy) {
}
// 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
// cannot be derived. Without this the screen would take a password and fail
// after deriving it. Reject stays available; the wallet is not touched.
// the address the approval was raised for belongs to a wallet whose key
// getSignerForAddress refuses. Without this the screen would take a password
// and fail after deriving it. Reject stays available; the wallet is not
// touched.
// Returns true when it gated.
function gateOnWalletDefect(errorId, buttonId, address) {
const owner = findWalletFor(address);
+10 -1
View File
@@ -21,6 +21,7 @@ const {
} = require("./helpers");
const { state, currentNetwork } = require("../../shared/state");
const { getSignerForAddress } = require("../../shared/wallet");
const { walletDefect } = require("../../shared/walletDefects");
const { decryptWithPassword } = require("../../shared/vault");
const { formatUsd, getPrice } = require("../../shared/prices");
const { getProvider } = require("../../shared/balances");
@@ -537,6 +538,15 @@ function init(_ctx) {
onViewLeave("confirm-tx", clearPassword);
$("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;
if (!password) {
showError(
@@ -546,7 +556,6 @@ function init(_ctx) {
return;
}
const wallet = state.wallets[state.selectedWallet];
let decryptedSecret;
hideError("confirm-tx-password-error");
+11 -5
View File
@@ -13,11 +13,17 @@ const NON_MASTER_XPRV = "non-master-xprv";
// 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
// m/44'/60'/0'/0 from a depth-0 key, and the pre-#210 path derived the same
// four levels as a relative path beneath whatever depth it was given. A master
// import therefore stores a depth-4 xpub and a depth-d import stores depth
// d + 4, which makes the stored xpub an exact read on the imported key's
// depth — and it is readable without the password, unlike the key itself.
// m/44'/60'/0'/0 from a depth-0 key, and the path before #210 (57959b7)
// derived the same four levels as a relative path beneath whatever depth it
// was given. A master import therefore stores a depth-4 xpub and a depth-d
// import stores depth d + 4, which makes the stored xpub an exact read on the
// 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 DEFECTS = {