The wallet row in Settings, next to the rename and delete actions that are
already per-wallet: a [recovery phrase] button rendered only on HD wallet
rows, opening a new show-phrase screen. The AddressDetail "more" menu was
the alternative and is wrong — the phrase belongs to the wallet, not to one
address, and AddressDetail already owns the per-address private key export.
Structure, password gate and warning treatment mirror src/popup/views/addressDetail.js:297-374 (ExportPrivKey).
How each security requirement is discharged
HD only.walletHasRecoveryPhrase() in src/shared/wallet.js is an
allowlist on type === "hd", so key and xprv are both excluded, and so
is any type added later. It gates the button in Settings and is re-checked
in show() and in the reveal handler, so reaching the screen by another
route still cannot produce a phrase.
Nothing in the DOM before unlock.show() renders only the wallet name
and the password prompt. decryptWithPassword is the only thing that
produces the phrase, and its result is written to #show-phrase-value only
on success. A wrong password sets a full-sentence error, leaves the value
node empty and keeps the result section hidden.
Cleared on leave, by any route. Views holding a secret register a
cleanup with showView() via onViewLeave() in src/popup/views/helpers.js. A clear wired only to "Back" would leak
through the Settings gear, which navigates away without touching that
button.
A decrypt still in flight when the screen is left is discarded. reveal() captures a generation counter that every clear() bumps, and
writes nothing if it has moved (or if the current view is no longer show-phrase). See the rework section below.
Never restorable. The phrase is never assigned to state, so it cannot
be persisted. RESTORABLE_VIEWS moves from src/popup/index.js to src/popup/restorableViews.js, unchanged, with show-phrase absent — the
popup entry point cannot be required outside a browser, so the exclusion
was untestable where it lived.
Never logged.showPhrase.js does not import src/shared/log.js at
all, and the failed-decrypt path reports a fixed sentence rather than the
caught error. Two unit tests pin both.
Blocking: the phrase was written into the DOM after the screen had been
left.reveal() awaited decryptWithPassword() and then wrote #show-phrase-value with no check that the view was still current, so a
decrypt in flight when the screen was left landed the phrase after clear() had run, with nothing scheduled to wipe again.
Fixed in src/popup/views/showPhrase.js: a module-level revealGeneration
counter, bumped by every clear(), is captured before the await; isCurrentReveal(generation) requires that counter to be unmoved, a wallet
to still be selected and state.currentView to still be show-phrase. The
success path and the failed-decrypt path both bail on it before touching the
DOM. A counter rather than a bare walletIndex === null check so that
leaving and re-entering for a different wallet during one decrypt also
discards the stale result.
Every other await in the view, audited.decryptWithPassword() is the
only one — grep -n "await\|\.then\|setTimeout\|Promise\|async" over src/popup/views/showPhrase.js returns that call and its enclosing async function reveal(), nothing else. show(), clear(), fail() and init() are fully synchronous. The copy handler calls navigator.clipboard.writeText() without awaiting it, but it reads the
phrase out of the DOM synchronously first and writes nothing back, so it has
no post-await write to guard. No module outside this view writes any show-phrase node: the only other references to that name are the entry in VIEWS, the Settings button that opens the screen, and the comment in restorableViews.js.
The probe, before and after. New e2e test 12 forces the interleaving the
reviewer used — both clicks dispatched inside one page task, since crypto_pwhash is synchronous and a human cannot interleave them once
libsodium's wasm is warm. It waits for the Reveal button to be re-enabled
(the same continuation that would have written the phrase) rather than
guessing at a duration, and prints its measurement on every run.
Same test, same commit, guard reverted:
ok 11 - leaving by the settings gear wipes it too (#161)
# probe: len=81 equalsPhrase=true resultHidden=false viewHidden=true
not ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
phrase still in the DOM after leaving mid-decrypt
# 12/13 tests passed
# FAILED
With the guard in place:
ok 11 - leaving by the settings gear wipes it too (#161)
# probe: len=0 equalsPhrase=false resultHidden=true viewHidden=true
ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
ok 13 - reopening the popup never lands on the phrase screen (#161)
# 13/13 tests passed
Minor: pushCurrentView() could orphan a stack entry. Fixed. The push
moves out of the Settings click handler and into showPhrase.show(), after
the type gate and immediately before showView(), so nothing is pushed on
the paths where show() returns without navigating.
Minor: show-phrase reaching the persisted state.viewStack. Not
changed here. The stack is restored verbatim by src/shared/state.js, so
this is the general "a non-restorable view can be a Back target" behaviour
rather than anything specific to this screen; fixing it means filtering the
restored stack, which changes navigation for export-privkey and every
other non-restorable view. As the review says, no secret is exposed — the
screen comes back empty with walletIndex === null. Left for its own issue
rather than widened into this one.
Pre-existing export-privkey equivalent. Out of scope here; tracked as #221.
Corrected claim. An earlier version of this description said the README RESTORABLE_VIEWS reference was a stale reference introduced by the Screen
Map rewrite. That attribution was wrong: the reference to src/popup/index.js was correct at b9bc226, and it is this PR that makes
it stale by moving the file. The README text is updated for that reason, not
as a fix to earlier work.
Test evidence
make check (unit, jest) covers the type gate, the RESTORABLE_VIEWS
exclusion and the absence of a logger path. The DOM behaviour is driven
against the real popup in a real Chrome by make test-e2e, which is where
this repo tests views — nine new e2e tests, all green on the rebased branch:
1..13
ok 5 - only an HD wallet is offered the recovery phrase action (#161)
ok 6 - a key wallet is not offered the recovery phrase action (#161)
ok 7 - the recovery phrase screen holds nothing before the password (#161)
ok 8 - a wrong password reveals nothing (#161)
ok 9 - the correct password reveals the full phrase, and nothing logs it (#161)
ok 10 - "Back" wipes the revealed phrase (#161)
ok 11 - leaving by the settings gear wipes it too (#161)
ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
ok 13 - reopening the popup never lands on the phrase screen (#161)
# 13/13 tests passed
The e2e tests assert against the wallet's real phrase, captured from the
creation flow: createWallet() now returns it. "Some twelve words" would
pass against the wrong wallet, and "nothing at all" would pass against a
screen that showed what it was meant to hide.
Each property was demonstrated red before it was satisfied, by breaking it
on the finished branch and re-running:
Predicate loosened to type !== "key": 2 failed, 151 passed — an xprv wallet does not, an unknown or missing wallet type does not.
show-phrase added to RESTORABLE_VIEWS: 1 failed, 152 passed — the recovery phrase screen is not restorable, Expected: false / Received: true.
A log.errorf() added to the failed-decrypt path: 2 failed, 151 passed — both logger tests.
Type gate removed from the Settings row and onViewLeave() replaced by a
clear on the "Back" button only: 10/12 — not ok 6 ... the key wallet was offered the recovery phrase action and not ok 11 ... phrase still in the DOM after leaving via the settings gear. Test 10 stayed green, which is the point: Back alone is not
enough.
Liveness guard reverted: not ok 12, quoted above.
No pre-fix demonstration exists for the wrong-password test, because before
this change there was no screen to enter a password into.
make check and make test-e2e
Both run on this head, rebased onto next at 86cdea5:
Test Suites: 10 passed, 10 total
Tests: 183 passed, 183 total
Linting...
All matched files use Prettier code style!
Checking formatting...
All matched files use Prettier code style!
# 13/13 tests passed
The repo's check / check (push) status is "Waiting to run" on this head, as
it is repo-wide; CI green is therefore unverified and the results above are
from local runs.
Docs
README Screen Map gains a ShowRecoveryPhrase entry in the format the map was
just rebuilt into, plus the two Settings lines that point at it, and the
Secret handling paragraph states the in-flight rule. The Navigation
paragraph's RESTORABLE_VIEWS reference now points at src/popup/restorableViews.js, the file this PR moves it to. The End-to-End
Tests section names the mid-decrypt case. README TODO checkbox ticked; TODO.md Completed Steps gains one line.
Closes [#161](https://git.eeqj.de/sneak/AutistMask/issues/161).
## Entry point
The wallet row in Settings, next to the rename and delete actions that are
already per-wallet: a `[recovery phrase]` button rendered only on HD wallet
rows, opening a new `show-phrase` screen. The AddressDetail "more" menu was
the alternative and is wrong — the phrase belongs to the wallet, not to one
address, and AddressDetail already owns the per-address private key export.
Structure, password gate and warning treatment mirror
`src/popup/views/addressDetail.js:297-374` (ExportPrivKey).
## How each security requirement is discharged
- **HD only.** `walletHasRecoveryPhrase()` in `src/shared/wallet.js` is an
allowlist on `type === "hd"`, so `key` and `xprv` are both excluded, and so
is any type added later. It gates the button in Settings and is re-checked
in `show()` and in the reveal handler, so reaching the screen by another
route still cannot produce a phrase.
- **Nothing in the DOM before unlock.** `show()` renders only the wallet name
and the password prompt. `decryptWithPassword` is the only thing that
produces the phrase, and its result is written to `#show-phrase-value` only
on success. A wrong password sets a full-sentence error, leaves the value
node empty and keeps the result section hidden.
- **Cleared on leave, by any route.** Views holding a secret register a
cleanup with `showView()` via `onViewLeave()` in
`src/popup/views/helpers.js`. A clear wired only to "Back" would leak
through the Settings gear, which navigates away without touching that
button.
- **A decrypt still in flight when the screen is left is discarded.**
`reveal()` captures a generation counter that every `clear()` bumps, and
writes nothing if it has moved (or if the current view is no longer
`show-phrase`). See the rework section below.
- **Never restorable.** The phrase is never assigned to `state`, so it cannot
be persisted. `RESTORABLE_VIEWS` moves from `src/popup/index.js` to
`src/popup/restorableViews.js`, unchanged, with `show-phrase` absent — the
popup entry point cannot be required outside a browser, so the exclusion
was untestable where it lived.
- **Never logged.** `showPhrase.js` does not import `src/shared/log.js` at
all, and the failed-decrypt path reports a fixed sentence rather than the
caught error. Two unit tests pin both.
## Rework after review
Against the findings in
[the review](https://git.eeqj.de/sneak/AutistMask/pulls/215#issuecomment-56935).
**Blocking: the phrase was written into the DOM after the screen had been
left.** `reveal()` awaited `decryptWithPassword()` and then wrote
`#show-phrase-value` with no check that the view was still current, so a
decrypt in flight when the screen was left landed the phrase *after*
`clear()` had run, with nothing scheduled to wipe again.
Fixed in `src/popup/views/showPhrase.js`: a module-level `revealGeneration`
counter, bumped by every `clear()`, is captured before the await;
`isCurrentReveal(generation)` requires that counter to be unmoved, a wallet
to still be selected and `state.currentView` to still be `show-phrase`. The
success path and the failed-decrypt path both bail on it before touching the
DOM. A counter rather than a bare `walletIndex === null` check so that
leaving and re-entering for a *different* wallet during one decrypt also
discards the stale result.
**Every other await in the view, audited.** `decryptWithPassword()` is the
only one — `grep -n "await\|\.then\|setTimeout\|Promise\|async"` over
`src/popup/views/showPhrase.js` returns that call and its enclosing
`async function reveal()`, nothing else. `show()`, `clear()`, `fail()` and
`init()` are fully synchronous. The copy handler calls
`navigator.clipboard.writeText()` without awaiting it, but it reads the
phrase out of the DOM synchronously first and writes nothing back, so it has
no post-await write to guard. No module outside this view writes any
`show-phrase` node: the only other references to that name are the entry in
`VIEWS`, the Settings button that opens the screen, and the comment in
`restorableViews.js`.
**The probe, before and after.** New e2e test 12 forces the interleaving the
reviewer used — both clicks dispatched inside one page task, since
`crypto_pwhash` is synchronous and a human cannot interleave them once
libsodium's wasm is warm. It waits for the Reveal button to be re-enabled
(the same continuation that would have written the phrase) rather than
guessing at a duration, and prints its measurement on every run.
Same test, same commit, guard reverted:
```
ok 11 - leaving by the settings gear wipes it too (#161)
# probe: len=81 equalsPhrase=true resultHidden=false viewHidden=true
not ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
phrase still in the DOM after leaving mid-decrypt
# 12/13 tests passed
# FAILED
```
With the guard in place:
```
ok 11 - leaving by the settings gear wipes it too (#161)
# probe: len=0 equalsPhrase=false resultHidden=true viewHidden=true
ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
ok 13 - reopening the popup never lands on the phrase screen (#161)
# 13/13 tests passed
```
**Minor: `pushCurrentView()` could orphan a stack entry.** Fixed. The push
moves out of the Settings click handler and into `showPhrase.show()`, after
the type gate and immediately before `showView()`, so nothing is pushed on
the paths where `show()` returns without navigating.
**Minor: `show-phrase` reaching the persisted `state.viewStack`.** Not
changed here. The stack is restored verbatim by `src/shared/state.js`, so
this is the general "a non-restorable view can be a Back target" behaviour
rather than anything specific to this screen; fixing it means filtering the
restored stack, which changes navigation for `export-privkey` and every
other non-restorable view. As the review says, no secret is exposed — the
screen comes back empty with `walletIndex === null`. Left for its own issue
rather than widened into this one.
**Pre-existing `export-privkey` equivalent.** Out of scope here; tracked as
[#221](https://git.eeqj.de/sneak/AutistMask/issues/221).
**Corrected claim.** An earlier version of this description said the README
`RESTORABLE_VIEWS` reference was a stale reference introduced by the Screen
Map rewrite. That attribution was wrong: the reference to
`src/popup/index.js` was correct at `b9bc226`, and it is *this* PR that makes
it stale by moving the file. The README text is updated for that reason, not
as a fix to earlier work.
## Test evidence
`make check` (unit, jest) covers the type gate, the `RESTORABLE_VIEWS`
exclusion and the absence of a logger path. The DOM behaviour is driven
against the real popup in a real Chrome by `make test-e2e`, which is where
this repo tests views — nine new e2e tests, all green on the rebased branch:
```
1..13
ok 5 - only an HD wallet is offered the recovery phrase action (#161)
ok 6 - a key wallet is not offered the recovery phrase action (#161)
ok 7 - the recovery phrase screen holds nothing before the password (#161)
ok 8 - a wrong password reveals nothing (#161)
ok 9 - the correct password reveals the full phrase, and nothing logs it (#161)
ok 10 - "Back" wipes the revealed phrase (#161)
ok 11 - leaving by the settings gear wipes it too (#161)
ok 12 - leaving while the decrypt is in flight reveals nothing (#161)
ok 13 - reopening the popup never lands on the phrase screen (#161)
# 13/13 tests passed
```
The e2e tests assert against the wallet's real phrase, captured from the
creation flow: `createWallet()` now returns it. "Some twelve words" would
pass against the wrong wallet, and "nothing at all" would pass against a
screen that showed what it was meant to hide.
Each property was demonstrated red before it was satisfied, by breaking it
on the finished branch and re-running:
- Predicate loosened to `type !== "key"`:
`2 failed, 151 passed` — `an xprv wallet does not`,
`an unknown or missing wallet type does not`.
- `show-phrase` added to `RESTORABLE_VIEWS`:
`1 failed, 152 passed` — `the recovery phrase screen is not restorable`,
`Expected: false / Received: true`.
- A `log.errorf()` added to the failed-decrypt path:
`2 failed, 151 passed` — both logger tests.
- Type gate removed from the Settings row and `onViewLeave()` replaced by a
clear on the "Back" button only:
`10/12` — `not ok 6 ... the key wallet was offered the recovery phrase
action` and `not ok 11 ... phrase still in the DOM after leaving via the
settings gear`. Test 10 stayed green, which is the point: Back alone is not
enough.
- Liveness guard reverted: `not ok 12`, quoted above.
No pre-fix demonstration exists for the wrong-password test, because before
this change there was no screen to enter a password into.
## `make check` and `make test-e2e`
Both run on this head, rebased onto `next` at `86cdea5`:
```
Test Suites: 10 passed, 10 total
Tests: 183 passed, 183 total
Linting...
All matched files use Prettier code style!
Checking formatting...
All matched files use Prettier code style!
```
```
# 13/13 tests passed
```
The repo's `check / check (push)` status is "Waiting to run" on this head, as
it is repo-wide; CI green is therefore unverified and the results above are
from local runs.
## Docs
README Screen Map gains a ShowRecoveryPhrase entry in the format the map was
just rebuilt into, plus the two Settings lines that point at it, and the
Secret handling paragraph states the in-flight rule. The Navigation
paragraph's `RESTORABLE_VIEWS` reference now points at
`src/popup/restorableViews.js`, the file this PR moves it to. The End-to-End
Tests section names the mid-decrypt case. README TODO checkbox ticked;
`TODO.md` Completed Steps gains one line.
A user who created a wallet in AutistMask and did not write the phrase
down had no way to retrieve it. Adds a "Show recovery phrase" action on
the wallet row in Settings, next to the per-wallet actions that already
live there, mirroring the per-address private key export in structure,
password gate and warning treatment.
The screen displays the secret that owns every address in the wallet, so:
- Only HD wallets are offered it. walletHasRecoveryPhrase() is an
allowlist on type "hd", so the key and xprv types — which have no
phrase at all — are excluded, as is any type added later.
- Nothing is decrypted and nothing enters the page until
decryptWithPassword accepts the password. A wrong password produces a
full-sentence error and leaves the value node empty.
- Leaving the screen wipes it by any route, not just "Back": views that
hold a secret register a cleanup with showView() via onViewLeave(),
which also covers the settings gear.
- The phrase is never assigned to state, so it cannot be persisted, and
the view is not in RESTORABLE_VIEWS — reopening the popup lands on
Home. That set moves to src/popup/restorableViews.js so the exclusion
can be asserted directly; the popup entry point cannot be required
outside a browser.
- The phrase cannot reach the logger: the view does not import
src/shared/log.js, and the failed-decrypt path reports a fixed
sentence rather than the caught error.
Tests: unit coverage for the type gate, the RESTORABLE_VIEWS exclusion
and the absence of any logger path; the DOM behaviour is driven against
the real popup in the e2e suite, which is where this repo tests views.
clawbot
self-assigned this 2026-08-11 14:39:20 +02:00
src/popup/views/showPhrase.js:83-93 — the phrase is written into the DOM after the screen has been left, and nothing wipes it afterwards.reveal() awaits decryptWithPassword() and then writes $("show-phrase-value").textContent = phrase and un-hides #show-phrase-result with no check that the view is still current. If the user leaves during the decrypt, clear() (the onViewLeave hook) has already run, so the write lands after the wipe and no further wipe is scheduled: the phrase sits in #show-phrase-value inside the hidden #view-show-phrase for the rest of the popup's life — through Settings, Home, Send — until the user re-enters and re-leaves this screen, or the popup closes. Demonstrated on this head commit in the pinned e2e container, leaving via the settings gear while the decrypt was in flight: # PROBE len=81 equalsPhrase=true resultHidden=false viewHidden=true — the wallet's real phrase, verbatim, five seconds after the user left the screen.
Reachability, stated plainly rather than overclaimed: libsodium's crypto_pwhash is synchronous, so once the wasm is warm the only suspension point is a microtask and a human click cannot interleave; the probe forces the interleave by dispatching both clicks in one task. The human-reachable window is a still-pending sodium.ready on the first vault use of that page load. The defect is that a screen whose entire contract is "leaving wipes it" performs its one secret-writing operation with no liveness check at all.
Acceptable: after the await, bail before touching the DOM if the screen has been left — e.g. return without writing when walletIndex === null (clear() nulls it) or state.currentView !== VIEW. Tests 10 and 11 pass today only because they leave after the reveal has completed; the guard needs a test that leaves during it.
Minor:
src/popup/views/settings.js:124-127: pushCurrentView() runs before showPhrase.show(idx), which can return without navigating (non-HD, missing wallet), orphaning an entry on state.viewStack. Not reachable through the UI today because the button renders for HD wallets only; push only when the view is actually shown.
Leaving via the settings gear pushes show-phrase onto the persisted state.viewStack. After the popup is reopened onto Settings, "Back" lands on the phrase screen with walletIndex === null and "Reveal" answers "No wallet is selected." No secret is exposed; it is a dead end.
Pre-existing, not this PR: export-privkey registers no onViewLeave, so leaving it by the settings gear leaves the private key in #export-privkey-value for the life of the popup. The new hook makes that a two-line fix — worth its own issue.
Checked and clean: type gate (allowlist on "hd", re-checked in show() and reveal(), no other caller); nothing in the markup before unlock; no logger import, no logger call, no phrase in any error message; RESTORABLE_VIEWS moved verbatim (same ten entries, same order, show-phrase and export-privkey absent); phrase never assigned to state, so saveState() cannot persist it; full-sentence wrong-password error revealing nothing; onViewLeave() fires only for the view that registers it; single commit on next, title ends (closes #161); no Claude/Anthropic references or attribution trailers; make check 159/159 and make test-e2e 12/12 green here on 2957601.
The repo check / check (push) status on 2957601 is still "Waiting to run", so CI green is unverified — that is separate from the rework above.
FAIL — `needs-rework`.
**`src/popup/views/showPhrase.js:83-93` — the phrase is written into the DOM after the screen has been left, and nothing wipes it afterwards.** `reveal()` awaits `decryptWithPassword()` and then writes `$("show-phrase-value").textContent = phrase` and un-hides `#show-phrase-result` with no check that the view is still current. If the user leaves during the decrypt, `clear()` (the `onViewLeave` hook) has already run, so the write lands *after* the wipe and no further wipe is scheduled: the phrase sits in `#show-phrase-value` inside the hidden `#view-show-phrase` for the rest of the popup's life — through Settings, Home, Send — until the user re-enters and re-leaves this screen, or the popup closes. Demonstrated on this head commit in the pinned e2e container, leaving via the settings gear while the decrypt was in flight: `# PROBE len=81 equalsPhrase=true resultHidden=false viewHidden=true` — the wallet's real phrase, verbatim, five seconds after the user left the screen.
Reachability, stated plainly rather than overclaimed: libsodium's `crypto_pwhash` is synchronous, so once the wasm is warm the only suspension point is a microtask and a human click cannot interleave; the probe forces the interleave by dispatching both clicks in one task. The human-reachable window is a still-pending `sodium.ready` on the first vault use of that page load. The defect is that a screen whose entire contract is "leaving wipes it" performs its one secret-writing operation with no liveness check at all.
Acceptable: after the await, bail before touching the DOM if the screen has been left — e.g. return without writing when `walletIndex === null` (`clear()` nulls it) or `state.currentView !== VIEW`. Tests 10 and 11 pass today only because they leave after the reveal has completed; the guard needs a test that leaves during it.
Minor:
- `src/popup/views/settings.js:124-127`: `pushCurrentView()` runs before `showPhrase.show(idx)`, which can return without navigating (non-HD, missing wallet), orphaning an entry on `state.viewStack`. Not reachable through the UI today because the button renders for HD wallets only; push only when the view is actually shown.
- Leaving via the settings gear pushes `show-phrase` onto the persisted `state.viewStack`. After the popup is reopened onto Settings, "Back" lands on the phrase screen with `walletIndex === null` and "Reveal" answers "No wallet is selected." No secret is exposed; it is a dead end.
- Pre-existing, not this PR: `export-privkey` registers no `onViewLeave`, so leaving it by the settings gear leaves the private key in `#export-privkey-value` for the life of the popup. The new hook makes that a two-line fix — worth its own issue.
Checked and clean: type gate (allowlist on `"hd"`, re-checked in `show()` and `reveal()`, no other caller); nothing in the markup before unlock; no logger import, no logger call, no phrase in any error message; `RESTORABLE_VIEWS` moved verbatim (same ten entries, same order, `show-phrase` and `export-privkey` absent); phrase never assigned to `state`, so `saveState()` cannot persist it; full-sentence wrong-password error revealing nothing; `onViewLeave()` fires only for the view that registers it; single commit on `next`, title ends ` (closes #161)`; no Claude/Anthropic references or attribution trailers; `make check` 159/159 and `make test-e2e` 12/12 green here on `2957601`.
The repo `check / check (push)` status on `2957601` is still "Waiting to run", so CI green is unverified — that is separate from the rework above.
PASS at c951028: six added interleavings all clean, make check 183/183 + prettier clean, make test-e2e 13/13 (20/20 with my probes), clean merge onto next at f455b0a with TODO.md retaining every landed entry, single commit authored clawbot ending (closes #161), no attribution trailers or vendor references.
Probes run, each demonstrated red with the guard reverted and green with it, so shipped test 12 is not vacuous (len=73 equalsPhrase=true reverted vs len=0 intact): leave and re-enter the SAME wallet mid-decrypt; leave and re-enter a DIFFERENT wallet (reverted, wallet 1's phrase landed verbatim on wallet 3's screen); two decrypts genuinely in flight at once (button force-enabled — the first reveal does disable it, so the UI cannot produce this unaided); a failed decrypt racing a re-entry (reverted, the stale failure stomped the live screen); popup closed mid-decrypt then reopened. In the re-entry cases viewHidden=false and walletIndex were both restored before the continuation ran, so revealGeneration was the only condition rejecting the write — the stated reason for a counter over a bare null check holds. clear() is the sole mutation of the counter and is reached by every entry (show()) and every leave (onViewLeave via showView()); no path leaves it unbumped. Await audit re-done independently: one await, one async, no .then/setTimeout/Promise/queueMicrotask in the view, all three addEventListener calls in init() and none inside reveal(); the only deferred helper reachable from this view is flashCopyFeedback(), which touches classes only.
Anomalies that pass anyway:
showPhrase.show() pushes the nav stack itself, unlike every other view (pushed by the caller). Deliberate and commented; settings.js:128 is the only caller, and no route into Settings gains a double- or missing-push.
e2e test 9 asserts click-to-copy by reading the title attribute only; the handler is correct by inspection but nothing clicks it.
The copy handler leaves the phrase on the system clipboard, which outlives the wipe. That is the issue's own definition of done, not a defect.
Leaving by the gear pushes show-phrase onto viewStack in-session as well as across a reopen, so Back from Settings lands on the empty screen. Same class as the tracked restored-stack item and identical for export-privkey; not counted here.
Disclosures: tracker CI ignored on instruction (runner misattributes unrelated jobs); script/lint is host prettier --check by this repo's own design rather than containerized, and was run through make check.
PASS at `c951028`: six added interleavings all clean, `make check` 183/183 + prettier clean, `make test-e2e` 13/13 (20/20 with my probes), clean merge onto `next` at `f455b0a` with `TODO.md` retaining every landed entry, single commit authored `clawbot` ending ` (closes #161)`, no attribution trailers or vendor references.
Probes run, each demonstrated red with the guard reverted and green with it, so shipped test 12 is not vacuous (`len=73 equalsPhrase=true` reverted vs `len=0` intact): leave and re-enter the SAME wallet mid-decrypt; leave and re-enter a DIFFERENT wallet (reverted, wallet 1's phrase landed verbatim on wallet 3's screen); two decrypts genuinely in flight at once (button force-enabled — the first reveal does disable it, so the UI cannot produce this unaided); a failed decrypt racing a re-entry (reverted, the stale failure stomped the live screen); popup closed mid-decrypt then reopened. In the re-entry cases `viewHidden=false` and `walletIndex` were both restored before the continuation ran, so `revealGeneration` was the only condition rejecting the write — the stated reason for a counter over a bare null check holds. `clear()` is the sole mutation of the counter and is reached by every entry (`show()`) and every leave (`onViewLeave` via `showView()`); no path leaves it unbumped. Await audit re-done independently: one `await`, one `async`, no `.then`/`setTimeout`/`Promise`/`queueMicrotask` in the view, all three `addEventListener` calls in `init()` and none inside `reveal()`; the only deferred helper reachable from this view is `flashCopyFeedback()`, which touches classes only.
Anomalies that pass anyway:
- `showPhrase.show()` pushes the nav stack itself, unlike every other view (pushed by the caller). Deliberate and commented; `settings.js:128` is the only caller, and no route into Settings gains a double- or missing-push.
- e2e test 9 asserts click-to-copy by reading the `title` attribute only; the handler is correct by inspection but nothing clicks it.
- The copy handler leaves the phrase on the system clipboard, which outlives the wipe. That is the issue's own definition of done, not a defect.
- Leaving by the gear pushes `show-phrase` onto `viewStack` in-session as well as across a reopen, so Back from Settings lands on the empty screen. Same class as the tracked restored-stack item and identical for `export-privkey`; not counted here.
Disclosures: tracker CI ignored on instruction (runner misattributes unrelated jobs); `script/lint` is host `prettier --check` by this repo's own design rather than containerized, and was run through `make check`.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #161.
Entry point
The wallet row in Settings, next to the rename and delete actions that are
already per-wallet: a
[recovery phrase]button rendered only on HD walletrows, opening a new
show-phrasescreen. The AddressDetail "more" menu wasthe alternative and is wrong — the phrase belongs to the wallet, not to one
address, and AddressDetail already owns the per-address private key export.
Structure, password gate and warning treatment mirror
src/popup/views/addressDetail.js:297-374(ExportPrivKey).How each security requirement is discharged
walletHasRecoveryPhrase()insrc/shared/wallet.jsis anallowlist on
type === "hd", sokeyandxprvare both excluded, and sois any type added later. It gates the button in Settings and is re-checked
in
show()and in the reveal handler, so reaching the screen by anotherroute still cannot produce a phrase.
show()renders only the wallet nameand the password prompt.
decryptWithPasswordis the only thing thatproduces the phrase, and its result is written to
#show-phrase-valueonlyon success. A wrong password sets a full-sentence error, leaves the value
node empty and keeps the result section hidden.
cleanup with
showView()viaonViewLeave()insrc/popup/views/helpers.js. A clear wired only to "Back" would leakthrough the Settings gear, which navigates away without touching that
button.
reveal()captures a generation counter that everyclear()bumps, andwrites nothing if it has moved (or if the current view is no longer
show-phrase). See the rework section below.state, so it cannotbe persisted.
RESTORABLE_VIEWSmoves fromsrc/popup/index.jstosrc/popup/restorableViews.js, unchanged, withshow-phraseabsent — thepopup entry point cannot be required outside a browser, so the exclusion
was untestable where it lived.
showPhrase.jsdoes not importsrc/shared/log.jsatall, and the failed-decrypt path reports a fixed sentence rather than the
caught error. Two unit tests pin both.
Rework after review
Against the findings in
the review.
Blocking: the phrase was written into the DOM after the screen had been
left.
reveal()awaiteddecryptWithPassword()and then wrote#show-phrase-valuewith no check that the view was still current, so adecrypt in flight when the screen was left landed the phrase after
clear()had run, with nothing scheduled to wipe again.Fixed in
src/popup/views/showPhrase.js: a module-levelrevealGenerationcounter, bumped by every
clear(), is captured before the await;isCurrentReveal(generation)requires that counter to be unmoved, a walletto still be selected and
state.currentViewto still beshow-phrase. Thesuccess path and the failed-decrypt path both bail on it before touching the
DOM. A counter rather than a bare
walletIndex === nullcheck so thatleaving and re-entering for a different wallet during one decrypt also
discards the stale result.
Every other await in the view, audited.
decryptWithPassword()is theonly one —
grep -n "await\|\.then\|setTimeout\|Promise\|async"oversrc/popup/views/showPhrase.jsreturns that call and its enclosingasync function reveal(), nothing else.show(),clear(),fail()andinit()are fully synchronous. The copy handler callsnavigator.clipboard.writeText()without awaiting it, but it reads thephrase out of the DOM synchronously first and writes nothing back, so it has
no post-await write to guard. No module outside this view writes any
show-phrasenode: the only other references to that name are the entry inVIEWS, the Settings button that opens the screen, and the comment inrestorableViews.js.The probe, before and after. New e2e test 12 forces the interleaving the
reviewer used — both clicks dispatched inside one page task, since
crypto_pwhashis synchronous and a human cannot interleave them oncelibsodium's wasm is warm. It waits for the Reveal button to be re-enabled
(the same continuation that would have written the phrase) rather than
guessing at a duration, and prints its measurement on every run.
Same test, same commit, guard reverted:
With the guard in place:
Minor:
pushCurrentView()could orphan a stack entry. Fixed. The pushmoves out of the Settings click handler and into
showPhrase.show(), afterthe type gate and immediately before
showView(), so nothing is pushed onthe paths where
show()returns without navigating.Minor:
show-phrasereaching the persistedstate.viewStack. Notchanged here. The stack is restored verbatim by
src/shared/state.js, sothis is the general "a non-restorable view can be a Back target" behaviour
rather than anything specific to this screen; fixing it means filtering the
restored stack, which changes navigation for
export-privkeyand everyother non-restorable view. As the review says, no secret is exposed — the
screen comes back empty with
walletIndex === null. Left for its own issuerather than widened into this one.
Pre-existing
export-privkeyequivalent. Out of scope here; tracked as#221.
Corrected claim. An earlier version of this description said the README
RESTORABLE_VIEWSreference was a stale reference introduced by the ScreenMap rewrite. That attribution was wrong: the reference to
src/popup/index.jswas correct atb9bc226, and it is this PR that makesit stale by moving the file. The README text is updated for that reason, not
as a fix to earlier work.
Test evidence
make check(unit, jest) covers the type gate, theRESTORABLE_VIEWSexclusion and the absence of a logger path. The DOM behaviour is driven
against the real popup in a real Chrome by
make test-e2e, which is wherethis repo tests views — nine new e2e tests, all green on the rebased branch:
The e2e tests assert against the wallet's real phrase, captured from the
creation flow:
createWallet()now returns it. "Some twelve words" wouldpass against the wrong wallet, and "nothing at all" would pass against a
screen that showed what it was meant to hide.
Each property was demonstrated red before it was satisfied, by breaking it
on the finished branch and re-running:
type !== "key":2 failed, 151 passed—an xprv wallet does not,an unknown or missing wallet type does not.show-phraseadded toRESTORABLE_VIEWS:1 failed, 152 passed—the recovery phrase screen is not restorable,Expected: false / Received: true.log.errorf()added to the failed-decrypt path:2 failed, 151 passed— both logger tests.onViewLeave()replaced by aclear on the "Back" button only:
10/12—not ok 6 ... the key wallet was offered the recovery phrase actionandnot ok 11 ... phrase still in the DOM after leaving via the settings gear. Test 10 stayed green, which is the point: Back alone is notenough.
not ok 12, quoted above.No pre-fix demonstration exists for the wrong-password test, because before
this change there was no screen to enter a password into.
make checkandmake test-e2eBoth run on this head, rebased onto
nextat86cdea5:The repo's
check / check (push)status is "Waiting to run" on this head, asit is repo-wide; CI green is therefore unverified and the results above are
from local runs.
Docs
README Screen Map gains a ShowRecoveryPhrase entry in the format the map was
just rebuilt into, plus the two Settings lines that point at it, and the
Secret handling paragraph states the in-flight rule. The Navigation
paragraph's
RESTORABLE_VIEWSreference now points atsrc/popup/restorableViews.js, the file this PR moves it to. The End-to-EndTests section names the mid-decrypt case. README TODO checkbox ticked;
TODO.mdCompleted Steps gains one line.A user who created a wallet in AutistMask and did not write the phrase down had no way to retrieve it. Adds a "Show recovery phrase" action on the wallet row in Settings, next to the per-wallet actions that already live there, mirroring the per-address private key export in structure, password gate and warning treatment. The screen displays the secret that owns every address in the wallet, so: - Only HD wallets are offered it. walletHasRecoveryPhrase() is an allowlist on type "hd", so the key and xprv types — which have no phrase at all — are excluded, as is any type added later. - Nothing is decrypted and nothing enters the page until decryptWithPassword accepts the password. A wrong password produces a full-sentence error and leaves the value node empty. - Leaving the screen wipes it by any route, not just "Back": views that hold a secret register a cleanup with showView() via onViewLeave(), which also covers the settings gear. - The phrase is never assigned to state, so it cannot be persisted, and the view is not in RESTORABLE_VIEWS — reopening the popup lands on Home. That set moves to src/popup/restorableViews.js so the exclusion can be asserted directly; the popup entry point cannot be required outside a browser. - The phrase cannot reach the logger: the view does not import src/shared/log.js, and the failed-decrypt path reports a fixed sentence rather than the caught error. Tests: unit coverage for the type gate, the RESTORABLE_VIEWS exclusion and the absence of any logger path; the DOM behaviour is driven against the real popup in the e2e suite, which is where this repo tests views.FAIL —
needs-rework.src/popup/views/showPhrase.js:83-93— the phrase is written into the DOM after the screen has been left, and nothing wipes it afterwards.reveal()awaitsdecryptWithPassword()and then writes$("show-phrase-value").textContent = phraseand un-hides#show-phrase-resultwith no check that the view is still current. If the user leaves during the decrypt,clear()(theonViewLeavehook) has already run, so the write lands after the wipe and no further wipe is scheduled: the phrase sits in#show-phrase-valueinside the hidden#view-show-phrasefor the rest of the popup's life — through Settings, Home, Send — until the user re-enters and re-leaves this screen, or the popup closes. Demonstrated on this head commit in the pinned e2e container, leaving via the settings gear while the decrypt was in flight:# PROBE len=81 equalsPhrase=true resultHidden=false viewHidden=true— the wallet's real phrase, verbatim, five seconds after the user left the screen.Reachability, stated plainly rather than overclaimed: libsodium's
crypto_pwhashis synchronous, so once the wasm is warm the only suspension point is a microtask and a human click cannot interleave; the probe forces the interleave by dispatching both clicks in one task. The human-reachable window is a still-pendingsodium.readyon the first vault use of that page load. The defect is that a screen whose entire contract is "leaving wipes it" performs its one secret-writing operation with no liveness check at all.Acceptable: after the await, bail before touching the DOM if the screen has been left — e.g. return without writing when
walletIndex === null(clear()nulls it) orstate.currentView !== VIEW. Tests 10 and 11 pass today only because they leave after the reveal has completed; the guard needs a test that leaves during it.Minor:
src/popup/views/settings.js:124-127:pushCurrentView()runs beforeshowPhrase.show(idx), which can return without navigating (non-HD, missing wallet), orphaning an entry onstate.viewStack. Not reachable through the UI today because the button renders for HD wallets only; push only when the view is actually shown.show-phraseonto the persistedstate.viewStack. After the popup is reopened onto Settings, "Back" lands on the phrase screen withwalletIndex === nulland "Reveal" answers "No wallet is selected." No secret is exposed; it is a dead end.export-privkeyregisters noonViewLeave, so leaving it by the settings gear leaves the private key in#export-privkey-valuefor the life of the popup. The new hook makes that a two-line fix — worth its own issue.Checked and clean: type gate (allowlist on
"hd", re-checked inshow()andreveal(), no other caller); nothing in the markup before unlock; no logger import, no logger call, no phrase in any error message;RESTORABLE_VIEWSmoved verbatim (same ten entries, same order,show-phraseandexport-privkeyabsent); phrase never assigned tostate, sosaveState()cannot persist it; full-sentence wrong-password error revealing nothing;onViewLeave()fires only for the view that registers it; single commit onnext, title ends(closes #161); no Claude/Anthropic references or attribution trailers;make check159/159 andmake test-e2e12/12 green here on2957601.The repo
check / check (push)status on2957601is still "Waiting to run", so CI green is unverified — that is separate from the rework above.2957601fcdtoad34fa8699ad34fa8699toc951028837PASS at
c951028: six added interleavings all clean,make check183/183 + prettier clean,make test-e2e13/13 (20/20 with my probes), clean merge ontonextatf455b0awithTODO.mdretaining every landed entry, single commit authoredclawbotending(closes #161), no attribution trailers or vendor references.Probes run, each demonstrated red with the guard reverted and green with it, so shipped test 12 is not vacuous (
len=73 equalsPhrase=truereverted vslen=0intact): leave and re-enter the SAME wallet mid-decrypt; leave and re-enter a DIFFERENT wallet (reverted, wallet 1's phrase landed verbatim on wallet 3's screen); two decrypts genuinely in flight at once (button force-enabled — the first reveal does disable it, so the UI cannot produce this unaided); a failed decrypt racing a re-entry (reverted, the stale failure stomped the live screen); popup closed mid-decrypt then reopened. In the re-entry casesviewHidden=falseandwalletIndexwere both restored before the continuation ran, sorevealGenerationwas the only condition rejecting the write — the stated reason for a counter over a bare null check holds.clear()is the sole mutation of the counter and is reached by every entry (show()) and every leave (onViewLeaveviashowView()); no path leaves it unbumped. Await audit re-done independently: oneawait, oneasync, no.then/setTimeout/Promise/queueMicrotaskin the view, all threeaddEventListenercalls ininit()and none insidereveal(); the only deferred helper reachable from this view isflashCopyFeedback(), which touches classes only.Anomalies that pass anyway:
showPhrase.show()pushes the nav stack itself, unlike every other view (pushed by the caller). Deliberate and commented;settings.js:128is the only caller, and no route into Settings gains a double- or missing-push.titleattribute only; the handler is correct by inspection but nothing clicks it.show-phraseontoviewStackin-session as well as across a reopen, so Back from Settings lands on the empty screen. Same class as the tracked restored-stack item and identical forexport-privkey; not counted here.Disclosures: tracker CI ignored on instruction (runner misattributes unrelated jobs);
script/lintis hostprettier --checkby this repo's own design rather than containerized, and was run throughmake check.c951028837to2c1f724545