sneak/AutistMask
1
0
Fork 0
Code Issues 50 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: a forgotten password permanently wedges the wallet — cannot delete it, cannot re-import the phrase #312

New Issue
Closed
opened 2026-08-20 12:00:07 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-20 12:00:07 +02:00
Collaborator
Copy Link

Found by the pre-1.0 deployability audit (#303). Blocker: a user holding their recovery phrase can still be locked out of the product with no route back.

  • src/popup/views/deleteWallet.js gates deletion on the password.
  • src/popup/views/addWallet.js:137-143 refuses a duplicate xpub: "This recovery phrase is already added".
  • There is no reset control anywhere in the UI.
  • The password hint (src/popup/index.html:158-161) says only "This password encrypts your recovery phrase on this device. You will need it to send funds." It never says the password cannot be recovered or reset.

Consequence

A user who forgets the password but does hold their recovery phrase cannot use it. They cannot delete the wallet (password-gated) and cannot re-import the phrase (duplicate-refused). Their only escape is destroying the extension's storage through browser internals, which takes every other wallet with it. The funds are recoverable in other software, but AutistMask has locked itself, and the product never warned that this was possible.

Definition of done

  • DeleteWallet offers a "I have lost my password" route that destroys the stored secret after a typed confirmation naming the wallet. No password may be required to discard a secret — requiring one protects nothing.
  • Or: re-import of a known xpub is allowed and re-encrypts under the new password. Either satisfies this, not both.
  • AddWallet states in one sentence that this password cannot be recovered or reset, and that the recovery phrase is the only backup.
  • Test: forget-password route removes the wallet and leaves the others intact.
  • make check green.
Found by the pre-1.0 deployability audit (https://git.eeqj.de/sneak/AutistMask/issues/303). **Blocker: a user holding their recovery phrase can still be locked out of the product with no route back.** - `src/popup/views/deleteWallet.js` gates deletion on the password. - `src/popup/views/addWallet.js:137-143` refuses a duplicate xpub: "This recovery phrase is already added". - There is no reset control anywhere in the UI. - The password hint (`src/popup/index.html:158-161`) says only *"This password encrypts your recovery phrase on this device. You will need it to send funds."* It never says the password cannot be recovered or reset. ## Consequence A user who forgets the password but **does** hold their recovery phrase cannot use it. They cannot delete the wallet (password-gated) and cannot re-import the phrase (duplicate-refused). Their only escape is destroying the extension's storage through browser internals, which takes every other wallet with it. The funds are recoverable in other software, but AutistMask has locked itself, and the product never warned that this was possible. ## Definition of done - [ ] DeleteWallet offers a "I have lost my password" route that destroys the stored secret after a typed confirmation naming the wallet. No password may be required to *discard* a secret — requiring one protects nothing. - [ ] Or: re-import of a known xpub is allowed and re-encrypts under the new password. Either satisfies this, not both. - [ ] AddWallet states in one sentence that this password cannot be recovered or reset, and that the recovery phrase is the only backup. - [ ] Test: forget-password route removes the wallet and leaves the others intact. - [ ] `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-20 12:00:07 +02:00
clawbot commented 2026-08-20 14:29:32 +02:00
Author
Collaborator
Copy Link

Plan. Taking the deletion route, not the re-import route.

Why: re-import would have to be built three times over (hd by xpub, xprv by xpub, key by address — a private-key wallet is duplicate-refused too, so an xpub-only fix leaves that user still wedged), it makes the user retype the recovery phrase into a live popup just to change a password, and it mutates a wallet in place. The deletion route reaches the same end state through code that already exists and is already tested: delete, then import again on AddWallet, which re-encrypts under the new password and re-runs scanForAddresses() to rediscover the used addresses. It also covers the case re-import does not — a user who has lost the password and simply wants the wallet gone.

Shape:

  • New view delete-wallet-lost-password, reached from an underlined "I have lost my password" control on delete-wallet-confirm. No password. Confirmation is typing the wallet's own name; the screen states plainly what is destroyed and that the recovery phrase is the only way back.
  • Not added to RESTORABLE_VIEWS — a screen whose button destroys a wallet is not one a popup reopened by accident should land on.
  • Both routes share one finishDelete(), so selection repair, site-permission cleanup and the AUTISTMASK_ACTIVE_CHANGED broadcast cannot drift apart.
  • The existing password route is left as it is. No new password gate is added.
  • AddWallet's password hint gains one sentence per import mode saying the password cannot be recovered or reset and what the only backup is, with the hint line given a reserved height so switching tabs cannot move the password fields.
  • Test drives the real view against a chrome.storage.local stub that structured-clones on both set and get, and asserts against what came back out of storage.
Plan. Taking the **deletion** route, not the re-import route. Why: re-import would have to be built three times over (`hd` by xpub, `xprv` by xpub, `key` by address — a private-key wallet is duplicate-refused too, so an xpub-only fix leaves that user still wedged), it makes the user retype the recovery phrase into a live popup just to change a password, and it mutates a wallet in place. The deletion route reaches the same end state through code that already exists and is already tested: delete, then import again on AddWallet, which re-encrypts under the new password and re-runs `scanForAddresses()` to rediscover the used addresses. It also covers the case re-import does not — a user who has lost the password and simply wants the wallet gone. Shape: - New view `delete-wallet-lost-password`, reached from an underlined "I have lost my password" control on `delete-wallet-confirm`. No password. Confirmation is typing the wallet's own name; the screen states plainly what is destroyed and that the recovery phrase is the only way back. - Not added to `RESTORABLE_VIEWS` — a screen whose button destroys a wallet is not one a popup reopened by accident should land on. - Both routes share one `finishDelete()`, so selection repair, site-permission cleanup and the `AUTISTMASK_ACTIVE_CHANGED` broadcast cannot drift apart. - The existing password route is left as it is. No new password gate is added. - AddWallet's password hint gains one sentence per import mode saying the password cannot be recovered or reset and what the only backup is, with the hint line given a reserved height so switching tabs cannot move the password fields. - Test drives the real view against a `chrome.storage.local` stub that structured-clones on both `set` and `get`, and asserts against what came back out of storage.
clawbot commented 2026-08-20 14:41:26 +02:00
Author
Collaborator
Copy Link

Built on the deletion route in #334; the reasoning against re-import is in the PR body.

DeleteWallet now offers "I have lost my password", leading to a new delete-wallet-lost-password screen that destroys the wallet after the user types its name back. No password anywhere in that path, and none added anywhere else. The screen says what is erased (the copy of the key on this device), what is not (anything on chain, the money at the addresses, the other wallets), and states in bold that without the recovery phrase written down the deletion loses everything the wallet holds. Both routes now run one finishDelete(), so the selection repair, site-permission cleanup and AUTISTMASK_ACTIVE_CHANGED broadcast are identical on each. The screen is excluded from RESTORABLE_VIEWS and registers an onViewLeave() cleanup.

AddWallet's password hint gained a sentence per import mode: the password cannot be recovered or reset, and the recovery phrase — or the private key, or the extended private key, according to the tab — is the only backup. The hint line reserves its height so the tabs cannot move the password fields.

Verification: make check green at exit 0, 827 tests over 40 suites, lint executed in the container rather than served from cache. The new suite drives the real view against a chrome.storage.local stub that structured-clones on both set and get and asserts against the storage read-back, with the stub's own non-aliasing pinned by the first test in the file. Two mutations run to show it is not vacuous: forcing the removal index to 0 fails 4 tests including "exactly the named wallet is destroyed", and deleting the await saveState() fails 5 — the second is the one an in-memory-only assertion would have missed.

Not run: the browser e2e suites, because script/test-e2e moves the autistmask-e2e-chrome image tag that is shared across sessions on this host. tests/popupElementIds.test.js in make check covers the new view's element ids statically.

Built on the deletion route in [#334](https://git.eeqj.de/sneak/AutistMask/pulls/334); the reasoning against re-import is in the PR body. DeleteWallet now offers "I have lost my password", leading to a new `delete-wallet-lost-password` screen that destroys the wallet after the user types its name back. No password anywhere in that path, and none added anywhere else. The screen says what is erased (the copy of the key on this device), what is not (anything on chain, the money at the addresses, the other wallets), and states in bold that without the recovery phrase written down the deletion loses everything the wallet holds. Both routes now run one `finishDelete()`, so the selection repair, site-permission cleanup and `AUTISTMASK_ACTIVE_CHANGED` broadcast are identical on each. The screen is excluded from `RESTORABLE_VIEWS` and registers an `onViewLeave()` cleanup. AddWallet's password hint gained a sentence per import mode: the password cannot be recovered or reset, and the recovery phrase — or the private key, or the extended private key, according to the tab — is the only backup. The hint line reserves its height so the tabs cannot move the password fields. Verification: `make check` green at exit 0, 827 tests over 40 suites, lint executed in the container rather than served from cache. The new suite drives the real view against a `chrome.storage.local` stub that structured-clones on both `set` and `get` and asserts against the storage read-back, with the stub's own non-aliasing pinned by the first test in the file. Two mutations run to show it is not vacuous: forcing the removal index to `0` fails 4 tests including "exactly the named wallet is destroyed", and deleting the `await saveState()` fails 5 — the second is the one an in-memory-only assertion would have missed. Not run: the browser e2e suites, because `script/test-e2e` moves the `autistmask-e2e-chrome` image tag that is shared across sessions on this host. `tests/popupElementIds.test.js` in `make check` covers the new view's element ids statically.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#312
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?