fix: explain a stored non-master xprv wallet instead of throwing at signing time (closes #234) #247

clawbot · 2026-08-12T10:27:11+02:00

clawbot commented

2026-08-12 10:27:11 +02:00

Closes #234.

The problem

#210 made extended-key import refuse a non-master key. A type: "xprv" wallet imported before that refusal existed still holds a key at depth > 0, and its addresses were derived by applying the Ethereum path beneath that key. Since #210 such a wallet throws inside getSignerForAddress — on the send screen, after the user has typed a password, with no explanation.

Detection without the password

An xprv wallet stores the neutered BIP-44 Ethereum node, four levels below the key that was imported: the current path derives the absolute m/44'/60'/0'/0 from a depth-0 key, and the pre-fix 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 — readable at wallet-list render time, with no decryption. New src/shared/walletDefects.js holds that check and the copy.

An xprv wallet whose xpub will not parse gets no defect rather than a guessed one: there is no basis in that case to tell the user their key was not a master key.

What the user sees

The wallet list renders a named explanation block under the wallet's name.
The "+" add-address button is withheld for that wallet.
Send (main view, address detail, address-token detail), dapp transaction approval, dapp message signing and Export Private Key all refuse before asking for a password. The approval screens disable the approve button and state the reason on the screen; Reject stays available.
getSignerForAddress remains the backstop and now refuses with a full sentence fitted to signing rather than to import.

Nothing is deleted or rewritten. The wallet, its addresses and its encrypted secret are untouched.

The copy

> This wallet's addresses were derived incorrectly.
>
> This wallet was imported from an extended private key that is not a master key. An earlier version applied the Ethereum derivation path beneath that key instead of from a master key, so the addresses listed here are not the ones that key produces under the standard path.
>
> Signing and sending are disabled for this wallet. The addresses do descend from the extended private key you imported, so anything they hold is still reachable by software that repeats the same non-standard derivation. Check them in a block explorer before deciding what to do.
>
> To see the addresses this key produces under the standard path, import the master extended private key, or the recovery phrase it came from, as a new wallet. Nothing here has been changed or removed, and this wallet stays until you delete it yourself.

The addresses genuinely descend from the key the user imported — the old code derived 44'/60'/0'/0 beneath it, which is a real derivation from a real key — so the copy neither promises the funds are safe nor implies anything was lost. It says where the addresses came from, that this build cannot sign for them, that they are reachable by software repeating the same derivation, and that the wallet is not being deleted. Tests assert the absence of "safe", "lost", "stolen" and "gone", and the absence of "seed phrase", "mnemonic", "passphrase" and "account".

Tests

tests/walletDefects.test.js, 22 tests. The fixtures are built the way each import path actually built them (the broken one via HDNodeWallet.fromExtendedKey(accountXprv).derivePath("44'/60'/0'/0"), since the module under test now refuses that input), and two tests pin the resulting xpub depths at 4 and 7 so the detection is anchored to real records rather than to an assumption.

Demonstrated failing first: with walletDefects.js stubbed to return null unconditionally and walletListHtml absent, make test reported 13 failed, 370 passed. After the fix, 383 passed.

Verification

make check green on the rebased branch: 15 suites, 383 tests passed, prettier --check clean for both lint and fmt-check. make build also run: 4 bundles emitted and verified autistmask-build-debug=off.

make test-e2e was not run; it needs a browser container and is not part of make check.

Closes [#234](https://git.eeqj.de/sneak/AutistMask/issues/234). ## The problem [#210](https://git.eeqj.de/sneak/AutistMask/issues/210) made extended-key import refuse a non-master key. A `type: "xprv"` wallet imported before that refusal existed still holds a key at depth > 0, and its addresses were derived by applying the Ethereum path *beneath* that key. Since [#210](https://git.eeqj.de/sneak/AutistMask/issues/210) such a wallet throws inside `getSignerForAddress` — on the send screen, after the user has typed a password, with no explanation. ## Detection without the password An `xprv` wallet stores the neutered BIP-44 Ethereum node, four levels below the key that was imported: the current path derives the absolute `m/44'/60'/0'/0` from a depth-0 key, and the pre-fix 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 — readable at wallet-list render time, with no decryption. New `src/shared/walletDefects.js` holds that check and the copy. An `xprv` wallet whose `xpub` will not parse gets no defect rather than a guessed one: there is no basis in that case to tell the user their key was not a master key. ## What the user sees - The wallet list renders a named explanation block under the wallet's name. - The "+" add-address button is withheld for that wallet. - Send (main view, address detail, address-token detail), dapp transaction approval, dapp message signing and Export Private Key all refuse before asking for a password. The approval screens disable the approve button and state the reason on the screen; Reject stays available. - `getSignerForAddress` remains the backstop and now refuses with a full sentence fitted to signing rather than to import. Nothing is deleted or rewritten. The wallet, its addresses and its encrypted secret are untouched. ## The copy > **This wallet's addresses were derived incorrectly.** > > This wallet was imported from an extended private key that is not a master key. An earlier version applied the Ethereum derivation path beneath that key instead of from a master key, so the addresses listed here are not the ones that key produces under the standard path. > > Signing and sending are disabled for this wallet. The addresses do descend from the extended private key you imported, so anything they hold is still reachable by software that repeats the same non-standard derivation. Check them in a block explorer before deciding what to do. > > To see the addresses this key produces under the standard path, import the master extended private key, or the recovery phrase it came from, as a new wallet. Nothing here has been changed or removed, and this wallet stays until you delete it yourself. The addresses genuinely descend from the key the user imported — the old code derived `44'/60'/0'/0` beneath it, which is a real derivation from a real key — so the copy neither promises the funds are safe nor implies anything was lost. It says where the addresses came from, that this build cannot sign for them, that they are reachable by software repeating the same derivation, and that the wallet is not being deleted. Tests assert the absence of "safe", "lost", "stolen" and "gone", and the absence of "seed phrase", "mnemonic", "passphrase" and "account". ## Tests `tests/walletDefects.test.js`, 22 tests. The fixtures are built the way each import path actually built them (the broken one via `HDNodeWallet.fromExtendedKey(accountXprv).derivePath("44'/60'/0'/0")`, since the module under test now refuses that input), and two tests pin the resulting `xpub` depths at 4 and 7 so the detection is anchored to real records rather than to an assumption. Demonstrated failing first: with `walletDefects.js` stubbed to return `null` unconditionally and `walletListHtml` absent, `make test` reported **13 failed, 370 passed**. After the fix, **383 passed**. ## Verification `make check` green on the rebased branch: 15 suites, 383 tests passed, `prettier --check` clean for both `lint` and `fmt-check`. `make build` also run: 4 bundles emitted and verified `autistmask-build-debug=off`. `make test-e2e` was not run; it needs a browser container and is not part of `make check`.

clawbot added the needs-review label 2026-08-12 10:27:11 +02:00

clawbot added 1 commit 2026-08-12 10:27:11 +02:00

fix: explain a stored non-master xprv wallet instead of throwing at signing time (closes #234 )

check / check (push) Successful in 28s

Details

0003a46ada

An xprv wallet imported before non-master keys were refused holds a key
whose depth is greater than zero. Its addresses were derived by applying
the Ethereum path beneath that key, so they are not the addresses the key
produces under the standard path, and signing for them now throws — on the
send screen, with no explanation.

Detect it at wallet-list render time instead. An xprv wallet stores the
neutered node four levels below the imported key, so a master import stores
a depth-4 xpub and a depth-d import stores depth d + 4; the stored xpub is
therefore an exact read on the imported key's depth and needs no password.

The wallet list renders a named explanation under the wallet's name, the
"+" button is withheld, and send, dapp transaction approval, dapp message
signing and private-key export all refuse before asking for a password.
getSignerForAddress remains the backstop and now says why in a sentence.

The copy states what is true and nothing more: the addresses do descend
from the key that was imported, so it neither promises the funds are safe
nor implies anything was lost. The wallet is not deleted or rewritten.

clawbot referenced this pull request

2026-08-12 10:27:20 +02:00

fix: an already-imported non-master xprv wallet will throw at signing time with no explanation #234

clawbot commented

2026-08-12 10:37:22 +02:00

PASS. Detection reproduced independently against real keys through both historical import paths (depth 0 stores xpub depth 4; depths 1/2/3/4/5 store 5/6/7/8/9 and are all detected; no false positive on any master import, and ethers refuses to serialize depth > 255 so there is no wrap-to-4 case); the copy's reachability claim is true — the stored addresses reproduce exactly from HDNodeWallet.fromExtendedKey(xprv).derivePath("44'/60'/0'/0") and carry private keys. make check 15 suites / 383 tests executed, make test-e2e 13/13 in the pinned container, script/cibuild green uncached, fast-forwardable onto origin/next at ba35282.

Disclosures and anomalies, none blocking:

Every UI gate survives deletion. Removing all six guards — home.js btn-send, addressDetail.js btn-send and btn-export-privkey, addressToken.js btn-address-token-send, and approval.js gateOnWalletDefect at both approval screens plus the two click-handler guards — leaves make test at 15 suites / 383 passed, and make test-e2e has no defective-wallet case. Only walletListHtml and the getSignerForAddress backstop carry coverage. The gates are correct as written and e2e proves the four touched views still load, but they have no regression signal.
The detection is exact for the released pre-#210 path only. Commit 7a7f9c5 stored the xpub with no derivation at all, so a wallet written by that build stores depth d: a master import would be falsely condemned (depth 0) and a depth-4 import missed. 7a7f9c5 and its fix 57959b7 share a commit timestamp and no tag contains 7a7f9c5, so this is unreachable from any release. Worth a clause in the walletDefects.js comment, which says "the pre-#210 path" as though there were one.
addressDetail.js btn-export-privkey comment: "There is no private key to export for an address this wallet cannot derive." There is one — deriving it from the stored xprv works and yields the displayed address. The app declines to. The gate is still right (the export screen's catch would otherwise report "Wrong password."), but the comment states something untrue.
showTxApproval does not reset btn-approve-tx.disabled before calling gateOnWalletDefect, where showSignApproval does. Unreachable, since each approval gets its own windows.create() document, but the two screens now differ.
Mutation testing required temporary edits in a private clone; tree restored clean, nothing committed or pushed.

**PASS.** Detection reproduced independently against real keys through both historical import paths (depth 0 stores xpub depth 4; depths 1/2/3/4/5 store 5/6/7/8/9 and are all detected; no false positive on any master import, and ethers refuses to serialize depth > 255 so there is no wrap-to-4 case); the copy's reachability claim is true — the stored addresses reproduce exactly from `HDNodeWallet.fromExtendedKey(xprv).derivePath("44'/60'/0'/0")` and carry private keys. `make check` 15 suites / 383 tests executed, `make test-e2e` 13/13 in the pinned container, `script/cibuild` green uncached, fast-forwardable onto `origin/next` at `ba35282`. Disclosures and anomalies, none blocking: - **Every UI gate survives deletion.** Removing all six guards — `home.js` btn-send, `addressDetail.js` btn-send and btn-export-privkey, `addressToken.js` btn-address-token-send, and `approval.js` `gateOnWalletDefect` at both approval screens plus the two click-handler guards — leaves `make test` at 15 suites / 383 passed, and `make test-e2e` has no defective-wallet case. Only `walletListHtml` and the `getSignerForAddress` backstop carry coverage. The gates are correct as written and e2e proves the four touched views still load, but they have no regression signal. - **The detection is exact for the released pre-[#210](https://git.eeqj.de/sneak/AutistMask/issues/210) path only.** Commit `7a7f9c5` stored the xpub with no derivation at all, so a wallet written by that build stores depth `d`: a master import would be falsely condemned (depth 0) and a depth-4 import missed. `7a7f9c5` and its fix `57959b7` share a commit timestamp and no tag contains `7a7f9c5`, so this is unreachable from any release. Worth a clause in the `walletDefects.js` comment, which says "the pre-#210 path" as though there were one. - `addressDetail.js` btn-export-privkey comment: "There is no private key to export for an address this wallet cannot derive." There is one — deriving it from the stored xprv works and yields the displayed address. The app declines to. The gate is still right (the export screen's `catch` would otherwise report "Wrong password."), but the comment states something untrue. - `showTxApproval` does not reset `btn-approve-tx.disabled` before calling `gateOnWalletDefect`, where `showSignApproval` does. Unreachable, since each approval gets its own `windows.create()` document, but the two screens now differ. - Mutation testing required temporary edits in a private clone; tree restored clean, nothing committed or pushed.

clawbot added needs-rebase and removed needs-review labels 2026-08-12 10:38:41 +02:00

clawbot force-pushed fix/issue-234-non-master-xprv-wallet from 0003a46ada to 0b2944a20e

2026-08-12 10:41:09 +02:00

Compare

clawbot merged commit bd4bdcafc7 into next

2026-08-12 10:41:49 +02:00

clawbot deleted branch fix/issue-234-non-master-xprv-wallet

2026-08-12 10:41:50 +02:00

clawbot referenced this pull request

2026-08-12 10:42:40 +02:00

test: the defective-wallet UI gates have no coverage — all six can be deleted with the suite still green #254

clawbot referenced this pull request