Compare commits

..

1 Commits

Author SHA1 Message Date
clawbot
ef82c62912 fix: count the network fee in the confirm-screen balance check (closes #154)
Some checks failed
check / check (push) Has been cancelled
The Send button was enabled whenever the amount alone fit the balance, so
a max-value ETH send passed the confirmation screen and failed at
broadcast, after the user had committed to it.

The arithmetic moves into src/shared/txValidation.js as a pure function
over 18-decimal fixed point: native ETH now requires amount + fee <=
balance, and an ERC-20 transfer requires the ETH balance to cover the fee
on top of the token check, reported as its own error. Validation re-runs
when the async estimate resolves; Send stays disabled while the estimate
is pending and when it fails, so an unknown fee is never treated as zero.
The fee messages are static elements that already reserve their space, so
nothing moves when the estimate lands.

The fee reserved is the one the node will actually require. The send pins
no fee fields, so ethers broadcasts a type-2 transaction and the node
validates it against value + gasLimit * maxFeePerGas; gating on gasPrice
would under-reserve by roughly gasLimit * baseFee and let through exactly
the broadcast failure this change exists to prevent. feeReserveWei()
derives that reserve, falling back to gasPrice only where no type-2
pricing exists. The reserve is read when the screen opens and the
broadcast derives its own, so a base fee that roughly doubles while the
user is at the password field can still outrun it: this turns a
deterministic failure on every max-value send into a rare one, not into
none.

The fee line shows both numbers rather than one - what the transfer is
expected to cost, and below it the larger amount reserved until it
confirms. Quoting only the reserve overstates the typical mainnet cost by
roughly double on every send; quoting only the estimate contradicts the
gate. The second line holds its space from the first paint.

validateTransfer() fails closed: a feeWei that is not a non-negative
bigint under FEE_KNOWN, any unrecognised feeStatus, and a negative amount
all block, rather than counting as a fee of zero or as an amount that
passes every comparison trivially.
2026-08-11 13:17:48 +00:00
18 changed files with 81 additions and 1160 deletions

View File

@@ -123,12 +123,8 @@ unavailable). The suite lives in `tests/e2e/` and is driven by
`playwright-core`, whose version must stay matched to the container's Playwright `playwright-core`, whose version must stay matched to the container's Playwright
version — the browsers ship inside the image. version — the browsers ship inside the image.
It covers popup load, wallet creation through the UI, the Add Token screen, the It covers popup load, wallet creation through the UI, the Add Token screen and
transaction detail screen for an ERC-20 transfer, and the recovery phrase screen the transaction detail screen for an ERC-20 transfer. All outbound network is
— which wallet types are offered it, that it holds nothing before the password
is accepted, that a wrong password reveals nothing, that leaving it by either
route wipes it — including a leave taken while the decrypt is still running —
and that reopening the popup does not land on it. All outbound network is
intercepted at the browser level and served from fixtures in intercepted at the browser level and served from fixtures in
`tests/e2e/network.js`, so the run is deterministic and fully offline; `tests/e2e/network.js`, so the run is deterministic and fully offline;
unrecognised outbound requests are reported as failures rather than silently unrecognised outbound requests are reported as failures rather than silently
@@ -410,11 +406,8 @@ runtime debug mode is on, or when the active network is a testnet. They are not
repeated in the element lists below. repeated in the element lists below.
Closing and reopening the popup returns to the screen the user was last on only Closing and reopening the popup returns to the screen the user was last on only
for the views listed in `RESTORABLE_VIEWS` (`src/popup/restorableViews.js`). for the views listed in `RESTORABLE_VIEWS` (`src/popup/index.js`). Every other
Every other screen falls back to Home. The screens that display a secret — screen, including ExportPrivKey, falls back to Home.
ExportPrivKey and ShowRecoveryPhrase — are deliberately absent from that list,
so the popup can never reopen onto one of them with no password prompt in front
of it.
#### Welcome (`welcome`) #### Welcome (`welcome`)
@@ -724,13 +717,12 @@ of it.
- **When**: User tapped the Settings gear. - **When**: User tapped the Settings gear.
- **Elements**: - **Elements**:
- "Back" button, "Settings" heading - "Back" button, "Settings" heading
- Wallets: one row per wallet with its name (tap to rename inline), a - Wallets: one row per wallet with its name (tap to rename inline) and an
`[recovery phrase]` button on HD wallets only, and an `[x]` delete button, `[x]` delete button, plus a "+ Add wallet" button
plus a "+ Add wallet" button
- Tracked Tokens: one row per tracked token with an `[x]` remove button, - Tracked Tokens: one row per tracked token with an `[x]` remove button,
plus a "+ Add token" button plus a "+ Add token" button
- Display: "Show tracked tokens with zero balance" checkbox, "UTC - Display: "Show tracked tokens with zero balance" checkbox and a Theme
Timestamps" checkbox, and a Theme selector (System / Light / Dark) selector (System / Light / Dark)
- Network: network selector (Ethereum Mainnet / Sepolia Testnet); switching - Network: network selector (Ethereum Mainnet / Sepolia Testnet); switching
resets the RPC and Blockscout endpoints to that network's defaults resets the RPC and Blockscout endpoints to that network's defaults
- Ethereum RPC: endpoint URL input + "Save" button (validated against - Ethereum RPC: endpoint URL input + "Save" button (validated against
@@ -741,6 +733,7 @@ of it.
- "Hide tokens with fewer than 1,000 holders" checkbox - "Hide tokens with fewer than 1,000 holders" checkbox
- "Hide transactions from detected fraud contracts" checkbox - "Hide transactions from detected fraud contracts" checkbox
- "Hide dust transactions below N gwei" checkbox + threshold input - "Hide dust transactions below N gwei" checkbox + threshold input
- "UTC Timestamps" checkbox
- Allowed Sites: list with remove buttons - Allowed Sites: list with remove buttons
- Denied Sites: list with remove buttons - Denied Sites: list with remove buttons
- About: project link, license, author, version, release date, and the - About: project link, license, author, version, release date, and the
@@ -750,7 +743,6 @@ of it.
- **Transitions**: - **Transitions**:
- "+ Add wallet" → **AddWallet** - "+ Add wallet" → **AddWallet**
- "+ Add token" → **SettingsAddToken** - "+ Add token" → **SettingsAddToken**
- `[recovery phrase]` on an HD wallet → **ShowRecoveryPhrase**
- `[x]` on a wallet → **DeleteWallet** - `[x]` on a wallet → **DeleteWallet**
- Tap wallet name → inline rename field (no screen change) - Tap wallet name → inline rename field (no screen change)
- `[x]` on a tracked token or a site → removes it in place (no screen - `[x]` on a tracked token or a site → removes it in place (no screen
@@ -758,33 +750,6 @@ of it.
- Ten clicks on the version → reveals the Debug well (no screen change) - Ten clicks on the version → reveals the Debug well (no screen change)
- "Back" (or Settings gear again) → previous screen (Home) - "Back" (or Settings gear again) → previous screen (Home)
#### ShowRecoveryPhrase (`show-phrase`)
- **When**: User tapped `[recovery phrase]` on a wallet row in Settings. HD
wallets only: key and xprv wallets have no recovery phrase, so their rows do
not offer the action at all.
- **Elements**:
- "Back" button, "Recovery Phrase" heading
- Wallet name
- Warning box stating that anyone holding these words can take everything in
the wallet, from any device, without the password
- Error line
- Password input + "Reveal" button, shown until the password is accepted
- The recovery phrase itself, in full and click-to-copy, shown only after a
correct password and in place of the password prompt
- **Transitions**:
- "Reveal" (correct password) → the phrase replaces the password prompt (no
screen change)
- "Reveal" (wrong password) → full-sentence error, nothing revealed (no
screen change)
- "Back" → previous screen (Settings)
- **Secret handling**: nothing is decrypted or written into the page until the
password is accepted; the phrase is never stored in state, and it is wiped
from the page whenever the screen is left by any route, including the Settings
gear. A decrypt still running when the screen is left is discarded rather than
written. The screen is not restorable, so reopening the popup lands on Home
rather than back on the phrase.
#### DeleteWallet (`delete-wallet-confirm`) #### DeleteWallet (`delete-wallet-confirm`)
- **When**: User tapped the `[x]` next to a wallet in Settings. - **When**: User tapped the `[x]` next to a wallet in Settings.
@@ -1227,7 +1192,7 @@ Currently supported:
- [x] Delete wallet (with confirmation) - [x] Delete wallet (with confirmation)
- [ ] Delete address from HD wallet (with confirmation) - [ ] Delete address from HD wallet (with confirmation)
- [x] Show wallet's recovery phrase (requires password) - [ ] Show wallet's recovery phrase (requires password)
### Transactions ### Transactions

15
TODO.md
View File

@@ -44,13 +44,6 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-11: `script/verify-build` now walks `dist/` NUL-delimited and asserts
`dist/` is a real directory, so a path with a trailing space or a newline can
no longer carry a debug marker past the unlisted-bundle check
([#223](https://git.eeqj.de/sneak/AutistMask/issues/223)).
- 2026-08-11: UTC Timestamps checkbox moved from the Token Spam Protection well
into Display, next to the theme selector
([#212](https://git.eeqj.de/sneak/AutistMask/issues/212)).
- 2026-08-11: Network fee counted in the confirmation-screen balance check for - 2026-08-11: Network fee counted in the confirmation-screen balance check for
both ETH and ERC-20 sends, reserving what the node actually charges a type-2 both ETH and ERC-20 sends, reserving what the node actually charges a type-2
transaction, with the arithmetic in a pure, unit-tested transaction, with the arithmetic in a pure, unit-tested
@@ -61,14 +54,6 @@ undefined identifiers, which is how
`src/shared/transactions.js` goes through one case-normalising helper so a `src/shared/transactions.js` goes through one case-normalising helper so a
checksummed genuine contract is no longer read as a spoof checksummed genuine contract is no longer read as a spoof
([#179](https://git.eeqj.de/sneak/AutistMask/issues/179)). ([#179](https://git.eeqj.de/sneak/AutistMask/issues/179)).
- 2026-08-11: Password-gated recovery phrase display for HD wallets, reached
from the wallet row in Settings, wiped on leaving the screen and excluded from
the views the popup can reopen onto
([#161](https://git.eeqj.de/sneak/AutistMask/issues/161)).
- 2026-08-11: Extended-key import hardened — the base58 checksum is now enforced
on every xprv and xpub, and a non-master key is refused with an explanation
instead of being derived beneath
([#210](https://git.eeqj.de/sneak/AutistMask/issues/210)).
- 2026-08-11: Policy compliance sweep — conditional verbose test rerun, local - 2026-08-11: Policy compliance sweep — conditional verbose test rerun, local
Tailwind binary instead of `npx`, `--frozen-lockfile` on `make install`, and Tailwind binary instead of `npx`, `--frozen-lockfile` on `make install`, and
the Makefile-only targets documented in the README the Makefile-only targets documented in the README

View File

@@ -362,16 +362,16 @@ Click the gear icon on the home screen to access settings:
- **Wallets**: Your wallets, and "+ Add wallet". - **Wallets**: Your wallets, and "+ Add wallet".
- **Tracked Tokens**: The ERC-20 tokens tracked across all addresses, and "+ Add - **Tracked Tokens**: The ERC-20 tokens tracked across all addresses, and "+ Add
token". token".
- **Display**: Toggle whether tracked tokens with zero balance are shown, switch - **Display**: Toggle whether tracked tokens with zero balance are shown, and
timestamps to UTC, and choose the theme (System, Light, or Dark). choose the theme (System, Light, or Dark).
- **Network**: Switch between Ethereum Mainnet and Sepolia Testnet. Switching - **Network**: Switch between Ethereum Mainnet and Sepolia Testnet. Switching
resets the RPC and Blockscout endpoints to that network's defaults. resets the RPC and Blockscout endpoints to that network's defaults.
- **Ethereum RPC**: Change the Ethereum node endpoint. Default is a public RPC. - **Ethereum RPC**: Change the Ethereum node endpoint. Default is a public RPC.
You can use your own node for maximum privacy. You can use your own node for maximum privacy.
- **Blockscout API**: Change the Blockscout instance used for token balances and - **Blockscout API**: Change the Blockscout instance used for token balances and
transaction history. You can use a self-hosted instance. transaction history. You can use a self-hosted instance.
- **Token Spam Protection**: Toggle individual scam filters and set the dust - **Token Spam Protection**: Toggle individual scam filters, set the dust
transaction threshold. transaction threshold, and switch timestamps to UTC.
- **Allowed Sites / Denied Sites**: View and manage web3 site permissions. - **Allowed Sites / Denied Sites**: View and manage web3 site permissions.
- **About**: License, author, version, release date, and a link to the commit - **About**: License, author, version, release date, and a link to the commit
this build came from. this build came from.

View File

@@ -22,18 +22,6 @@ set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Absolute path to this script, resolved before anything cd's anywhere.
# check_unlisted_bundles re-invokes it through xargs, and $0 on its own may be
# relative to a directory we are about to leave.
SELF="$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0")"
# Internal re-entry flag; see scan_dist_paths.
SCAN_FLAG="--scan-dist-paths"
# A literal newline, for the is_listed guard.
NEWLINE='
'
MANIFEST="dist/constants-bundles.txt" MANIFEST="dist/constants-bundles.txt"
MARKER_ON="autistmask-build-debug=on" MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off" MARKER_OFF="autistmask-build-debug=off"
@@ -41,20 +29,11 @@ MARKER_OFF="autistmask-build-debug=off"
# Set by read_marker. # Set by read_marker.
MARKER="" MARKER=""
# Temporary file holding the NUL-delimited dist/ listing, removed by the EXIT
# trap because fail() exits from wherever it is called.
LISTING=""
fail() { fail() {
echo "verify-build: FAIL: $*" >&2 echo "verify-build: FAIL: $*" >&2
exit 1 exit 1
} }
cleanup() {
[ -z "$LISTING" ] || rm -f "$LISTING"
}
trap cleanup EXIT
# Is the literal $1 present in the file $2? Match (grep exit 0) and no-match # Is the literal $1 present in the file $2? Match (grep exit 0) and no-match
# (exit 1) are answers about the emitted output. Anything else (exit 2: the # (exit 1) are answers about the emitted output. Anything else (exit 2: the
# file could not be read) is not an answer at all, and must not be reported as # file could not be read) is not an answer at all, and must not be reported as
@@ -79,17 +58,7 @@ has_marker() {
# manifest could not be read and is not an answer at all. Without this, an # manifest could not be read and is not an answer at all. Without this, an
# unreadable manifest reads as "this file is not listed" and every emitted # unreadable manifest reads as "this file is not listed" and every emitted
# bundle gets reported as an unlisted one. # bundle gets reported as an unlisted one.
#
# A path containing a newline is answered without asking grep, because grep
# would read the pattern as two patterns and report a match on either. That is
# how such a path escaped this check even once the walk stopped splitting it:
# the half before the newline matched a listed line and the file was skipped.
# The manifest is line-delimited, so it cannot name such a path at all, and
# "not listed" is the only true answer.
is_listed() { is_listed() {
case "$1" in
*"$NEWLINE"*) return 1 ;;
esac
_il_status=0 _il_status=0
grep -q -x -F -e "$1" -- "$MANIFEST" || _il_status=$? grep -q -x -F -e "$1" -- "$MANIFEST" || _il_status=$?
case "$_il_status" in case "$_il_status" in
@@ -148,66 +117,35 @@ read_marker() {
# an endsWith(".js") test; repeating that literal here would mean a bundle # an endsWith(".js") test; repeating that literal here would mean a bundle
# emitted under some other extension escaped the manifest AND this check at # emitted under some other extension escaped the manifest AND this check at
# once, which is the correlated blind spot the two-source design exists to # once, which is the correlated blind spot the two-source design exists to
# avoid. Every regular file and every symlink under dist/ is searched — that # avoid. Every file under dist/ is searched, so build.js's filter is the only
# is the whole of what a build emits — so build.js's filter is the only place # place the assumption lives and this check is what catches it being wrong.
# the assumption lives and this check is what catches it being wrong.
# #
# That claim only holds if the walk is exhaustive and every name survives it # That claim only holds if the walk is exhaustive, so two things are enforced
# intact, so four things are enforced here rather than assumed: # here rather than assumed:
# #
# - the walk is NUL-delimited and the paths reach the check as arguments, so
# no name can be reshaped on the way in. Read line by line, a name with a
# trailing space lost it to read's field splitting and the remnant then
# matched a manifest line, and a name containing a newline arrived as a
# listed path plus an empty one. Both left a marker-carrying, unlisted file
# unchecked while the script still reported success. Delivering such a name
# intact is only half of it; is_listed also has to keep it out of grep's
# pattern, for the same reason.
# - find's exit status is checked. A subtree it cannot descend is reported on # - find's exit status is checked. A subtree it cannot descend is reported on
# stderr and then simply missing from the listing, so an unchecked status # stderr and then simply missing from the listing, so an unchecked status
# turns "could not look" into "nothing was there" — the same conflation # turns "could not look" into "nothing was there" — the same conflation
# has_marker exists to prevent. The status cannot be read off a pipeline, # has_marker exists to prevent. The status cannot be read off a pipeline
# so the listing lands in a file that xargs then reads back. # ending in sort, so the sort is a separate step.
# - symlinks are walked too (-type l), not skipped. A marker-carrying bundle # - symlinks are walked too (-type l), not skipped. A marker-carrying bundle
# reachable under an unlisted path in dist/ is a stale manifest whether the # reachable under an unlisted path in dist/ is a stale manifest whether the
# path is a link or a file, and grep reads through the link. A link that # path is a link or a file, and grep reads through the link. A link that
# cannot be read through — dangling, or pointing at a directory — fails # cannot be read through — dangling, or pointing at a directory — fails
# hard via has_marker's exit-2 path, which is the fail-closed answer: the # hard via has_marker's exit-2 path, which is the fail-closed answer: the
# build emits neither, so their DEBUG state is unproven, not fine. # build emits neither, so their DEBUG state is unproven, not fine.
# - dist/ itself must be a directory and not a symlink, which main asserts
# before anything reads through it. find does not follow a symlink named on
# its own command line, so a linked dist/ collapses this walk to one entry
# and cross-checks nothing.
#
# Types other than regular files and symlinks are left out on purpose: a build
# emits none of them, and grep on a fifo would hang rather than fail.
check_unlisted_bundles() { check_unlisted_bundles() {
LISTING="$(mktemp "${TMPDIR:-/tmp}/verify-build-dist.XXXXXX")" ||
fail "could not create a temporary file for the dist/ listing, so the
tree was never walked. Refusing to report success."
_find_status=0 _find_status=0
find dist \( -type f -o -type l \) -print0 >"$LISTING" || _find_status=$? _listing="$(find dist \( -type f -o -type l \) -print)" || _find_status=$?
[ "$_find_status" -eq 0 ] || [ "$_find_status" -eq 0 ] ||
fail "find exited $_find_status enumerating dist/, so part of the tree fail "find exited $_find_status enumerating dist/, so part of the tree
was never walked and nothing was established about the files in it. Any was never walked and nothing was established about the files in it. Any
unlisted bundle there went unchecked. That is a permissions or I/O fault on unlisted bundle there went unchecked. That is a permissions or I/O fault on
the artifact, not a stale manifest. Refusing to report success." the artifact, not a stale manifest. Refusing to report success."
_listing="$(printf '%s\n' "$_listing" | sort)"
_scan_status=0 while read -r _file; do
xargs -0 "$SELF" "$SCAN_FLAG" <"$LISTING" || _scan_status=$? [ -n "$_file" ] || continue
[ "$_scan_status" -eq 0 ] ||
fail "the unlisted-bundle scan exited $_scan_status: either a path
under dist/ failed the check reported above, or the scan could not be run
at all. Refusing to report success."
}
# The per-path half of check_unlisted_bundles. It runs in a re-invocation of
# this script, so it uses the same is_listed and has_marker as the rest of the
# file rather than a second copy of them that could drift. Paths arrive as
# arguments and are never split, joined or trimmed.
scan_dist_paths() {
for _file in "$@"; do
if is_listed "$_file"; then if is_listed "$_file"; then
continue continue
fi fi
@@ -216,7 +154,9 @@ scan_dist_paths() {
fail "$_file carries a debug marker but is absent from $MANIFEST, fail "$_file carries a debug marker but is absent from $MANIFEST,
so the manifest no longer describes the emitted bundles." so the manifest no longer describes the emitted bundles."
fi fi
done done <<EOF
$_listing
EOF
} }
# The requested mode, read from our own environment using build.js's exact # The requested mode, read from our own environment using build.js's exact
@@ -233,32 +173,9 @@ expected_marker() {
main() { main() {
cd "$ROOT" cd "$ROOT"
# Internal re-entry from check_unlisted_bundles' xargs. Not part of the
# command-line interface: nothing else invokes it, and it is a distinct
# entry point rather than a mode flag threaded through the checks below.
if [ "${1-}" = "$SCAN_FLAG" ]; then
shift
scan_dist_paths "$@"
return 0
fi
expected="$(expected_marker)" expected="$(expected_marker)"
echo "Verifying emitted bundles (expecting $expected)..." echo "Verifying emitted bundles (expecting $expected)..."
# Asserted here rather than left to grep. A symlinked dist/ used to fail
# only because GNU grep exits 2 on a directory, so check_unlisted_bundles'
# single entry hit has_marker's I/O path by luck; under a grep that exits 1
# instead, the whole cross-check would have collapsed into a pass.
if [ -h dist ]; then
fail "dist is a symlink, not a directory. find does not follow a
symlink named on its own command line, so the unlisted-bundle cross-check
would see one entry instead of the emitted tree and establish nothing about
it. Refusing to report success."
fi
[ -d dist ] ||
fail "dist is not a directory, so there is no emitted tree to verify.
build.js writes it; run make build first."
[ -f "$MANIFEST" ] || [ -f "$MANIFEST" ] ||
fail "$MANIFEST is missing. build.js writes it at the end of a fail "$MANIFEST is missing. build.js writes it at the end of a
successful build; run make build first." successful build; run make build first."

View File

@@ -136,9 +136,7 @@
<div id="add-wallet-section-xprv" class="hidden"> <div id="add-wallet-section-xprv" class="hidden">
<p class="mb-2"> <p class="mb-2">
Paste your extended private key (xprv) below. This will Paste your extended private key (xprv) below. This will
import the HD wallet and scan for used addresses. It import the HD wallet and scan for used addresses.
must be the master key for the wallet; an account-level
or child key is not supported.
</p> </p>
<div class="mb-2"> <div class="mb-2">
<input <input
@@ -904,12 +902,6 @@
/> />
Show tracked tokens with zero balance Show tracked tokens with zero balance
</label> </label>
<label
class="text-xs flex items-center gap-1 cursor-pointer mb-2"
>
<input type="checkbox" id="settings-utc-timestamps" />
UTC Timestamps
</label>
<div class="text-xs flex items-center gap-1"> <div class="text-xs flex items-center gap-1">
<label for="settings-theme">Theme:</label> <label for="settings-theme">Theme:</label>
<select <select
@@ -1020,6 +1012,12 @@
/> />
<span class="text-xs text-muted">gwei</span> <span class="text-xs text-muted">gwei</span>
</div> </div>
<label
class="text-xs flex items-center gap-1 cursor-pointer mb-1"
>
<input type="checkbox" id="settings-utc-timestamps" />
UTC Timestamps
</label>
</div> </div>
<div class="bg-well p-3 mx-1 mb-3"> <div class="bg-well p-3 mx-1 mb-3">
@@ -1133,52 +1131,6 @@
</button> </button>
</div> </div>
<!-- ============ SHOW RECOVERY PHRASE ============ -->
<div id="view-show-phrase" class="view hidden">
<button
id="btn-show-phrase-back"
class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer mb-2"
>
&lt; Back
</button>
<h2 class="font-bold mb-1">Recovery Phrase</h2>
<p class="text-xs mb-3" id="show-phrase-wallet-name"></p>
<div
class="text-xs mb-3 border border-border border-dashed p-2"
>
Anyone who has these words can take every coin and token in
this wallet, from any device, without your password. Never
type them into a website and never show them to anyone.
</div>
<div
id="show-phrase-flash"
class="text-xs text-red-500 mb-2 min-h-[1.25rem]"
style="visibility: hidden"
></div>
<div id="show-phrase-password-section" class="mb-2">
<label class="block mb-1">Password</label>
<input
type="password"
id="show-phrase-password"
class="border border-border p-1 w-full font-mono text-sm bg-bg text-fg"
placeholder="Enter your password to continue"
/>
<button
id="btn-show-phrase-reveal"
class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer mt-2"
>
Reveal
</button>
</div>
<div id="show-phrase-result" class="hidden">
<div
id="show-phrase-value"
class="bg-danger-well rounded p-2 font-mono text-xs break-all cursor-pointer mb-1"
title="Click to copy"
></div>
</div>
</div>
<!-- ============ SETTINGS: ADD TOKEN ============ --> <!-- ============ SETTINGS: ADD TOKEN ============ -->
<div id="view-settings-addtoken" class="view hidden"> <div id="view-settings-addtoken" class="view hidden">
<button <button

View File

@@ -15,10 +15,6 @@ const {
clearViewStack, clearViewStack,
} = require("./views/helpers"); } = require("./views/helpers");
const { applyTheme } = require("./theme"); const { applyTheme } = require("./theme");
// Views that can be fully re-rendered from persisted state. All others fall
// back to the nearest restorable parent; see the module for why the
// secret-bearing views are absent.
const { RESTORABLE_VIEWS } = require("./restorableViews");
const home = require("./views/home"); const home = require("./views/home");
const welcome = require("./views/welcome"); const welcome = require("./views/welcome");
@@ -103,6 +99,21 @@ const ctx = {
}, },
}; };
// Views that can be fully re-rendered from persisted state.
// All others fall back to the nearest restorable parent.
const RESTORABLE_VIEWS = new Set([
"main",
"address",
"address-token",
"receive",
"settings",
"settings-addtoken",
"confirm-tx",
"transaction",
"success-tx",
"error-tx",
]);
function needsAddress(view) { function needsAddress(view) {
return ( return (
view === "address" || view === "address" ||

View File

@@ -1,29 +0,0 @@
// Views the popup may reopen onto.
//
// The popup persists the current view so that reopening the toolbar popup
// lands the user back where they were. Only views that can be fully
// re-rendered from persisted state belong here; every other view falls back
// to the nearest restorable parent (src/popup/index.js restoreView()).
//
// A view that displays a secret must NEVER be listed. Restoring onto one
// would put a private key or a recovery phrase on screen with no password
// prompt in front of it, on a popup the user may have reopened by accident.
// That is why "export-privkey" and "show-phrase" are absent.
//
// Kept in its own module, with no dependencies, so tests can assert the
// exclusion directly rather than trusting a reading of the popup entry
// point, which cannot be required outside a browser.
const RESTORABLE_VIEWS = new Set([
"main",
"address",
"address-token",
"receive",
"settings",
"settings-addtoken",
"confirm-tx",
"transaction",
"success-tx",
"error-tx",
]);
module.exports = { RESTORABLE_VIEWS };

View File

@@ -6,7 +6,6 @@ const {
addressFromPrivateKey, addressFromPrivateKey,
hdWalletFromXprv, hdWalletFromXprv,
isValidXprv, isValidXprv,
isMasterExtendedKey,
} = require("../../shared/wallet"); } = require("../../shared/wallet");
const { encryptWithPassword } = require("../../shared/vault"); const { encryptWithPassword } = require("../../shared/vault");
const { state, saveState } = require("../../shared/state"); const { state, saveState } = require("../../shared/state");
@@ -214,25 +213,14 @@ async function importXprvKey(ctx) {
return; return;
} }
if (!isValidXprv(xprv)) { if (!isValidXprv(xprv)) {
showFlash( showFlash("Invalid extended private key.");
"That extended private key is not valid. Please check it and try again.",
);
return;
}
if (!isMasterExtendedKey(xprv)) {
showFlash(
"That is an account-level or child key, which cannot be imported. " +
"Please paste the master extended private key for the wallet.",
);
return; return;
} }
let result; let result;
try { try {
result = hdWalletFromXprv(xprv); result = hdWalletFromXprv(xprv);
} catch (e) { } catch (e) {
showFlash( showFlash("Invalid extended private key.");
"That extended private key is not valid. Please check it and try again.",
);
return; return;
} }
const { xpub, firstAddress } = result; const { xpub, firstAddress } = result;

View File

@@ -345,9 +345,8 @@ async function estimateGas(txInfo) {
"up to " + formatFeeEth(gasCostWei) + " reserved"; "up to " + formatFeeEth(gasCostWei) + " reserved";
setVisible("confirm-fee-reserve", true); setVisible("confirm-fee-reserve", true);
} else { } else {
// No spread to report: either there is no estimate, or the node // No spread to report: a network with no type-2 pricing charges
// quotes a gas price at or above maxFeePerGas, so the expected // exactly what is reserved.
// cost is not below the reserve. Show the reserve alone.
$("confirm-fee-amount").textContent = valueWithUsd( $("confirm-fee-amount").textContent = valueWithUsd(
formatFeeEth(gasCostWei), formatFeeEth(gasCostWei),
usd(gasCostWei), usd(gasCostWei),

View File

@@ -31,20 +31,8 @@ const VIEWS = [
"approve-tx", "approve-tx",
"approve-sign", "approve-sign",
"export-privkey", "export-privkey",
"show-phrase",
]; ];
// Cleanup callbacks for views that hold a secret in the DOM. The view
// registers one for itself and showView() runs it whenever that view is
// navigated away from, so the secret is wiped no matter which control
// caused the navigation — "Back", the settings gear, or a jump from
// anywhere else. A per-button clear would only cover the one path.
const viewLeaveHandlers = new Map();
function onViewLeave(name, fn) {
viewLeaveHandlers.set(name, fn);
}
function $(id) { function $(id) {
return document.getElementById(id); return document.getElementById(id);
} }
@@ -62,11 +50,6 @@ function hideError(id) {
} }
function showView(name) { function showView(name) {
const leaving = state.currentView;
if (leaving && leaving !== name) {
const onLeave = viewLeaveHandlers.get(leaving);
if (onLeave) onLeave();
}
for (const v of VIEWS) { for (const v of VIEWS) {
const el = document.getElementById(`view-${v}`); const el = document.getElementById(`view-${v}`);
if (el) { if (el) {
@@ -448,12 +431,10 @@ function flashCopyFeedback(el) {
} }
module.exports = { module.exports = {
VIEWS,
$, $,
showError, showError,
hideError, hideError,
showView, showView,
onViewLeave,
updateDebugBanner, updateDebugBanner,
setRenderMain, setRenderMain,
pushCurrentView, pushCurrentView,

View File

@@ -14,8 +14,6 @@ const { NETWORKS, SUPPORTED_CHAIN_IDS } = require("../../shared/networks");
const { onChainSwitch } = require("../../shared/chainSwitch"); const { onChainSwitch } = require("../../shared/chainSwitch");
const { log, debugFetch, setRuntimeDebug } = require("../../shared/log"); const { log, debugFetch, setRuntimeDebug } = require("../../shared/log");
const deleteWallet = require("./deleteWallet"); const deleteWallet = require("./deleteWallet");
const showPhrase = require("./showPhrase");
const { walletHasRecoveryPhrase } = require("../../shared/wallet");
const { const {
BUILD_VERSION, BUILD_VERSION,
BUILD_LICENSE, BUILD_LICENSE,
@@ -101,14 +99,7 @@ function renderWalletListSettings() {
const name = escapeHtml(wallet.name || "Wallet " + (idx + 1)); const name = escapeHtml(wallet.name || "Wallet " + (idx + 1));
html += `<div class="flex justify-between items-center text-xs py-1 border-b border-border-light">`; html += `<div class="flex justify-between items-center text-xs py-1 border-b border-border-light">`;
html += `<span class="settings-wallet-name cursor-pointer underline decoration-dashed" data-idx="${idx}">${name}</span>`; html += `<span class="settings-wallet-name cursor-pointer underline decoration-dashed" data-idx="${idx}">${name}</span>`;
html += `<span class="flex items-center gap-1 flex-shrink-0">`;
// Key and xprv wallets have no recovery phrase, so they are never
// offered the action at all.
if (walletHasRecoveryPhrase(wallet)) {
html += `<button class="btn-show-phrase border border-border px-1 hover:bg-fg hover:text-bg cursor-pointer" data-idx="${idx}" title="Show recovery phrase">[recovery phrase]</button>`;
}
html += `<button class="btn-delete-wallet border border-border px-1 hover:bg-fg hover:text-bg cursor-pointer" data-idx="${idx}">[x]</button>`; html += `<button class="btn-delete-wallet border border-border px-1 hover:bg-fg hover:text-bg cursor-pointer" data-idx="${idx}">[x]</button>`;
html += `</span>`;
html += `</div>`; html += `</div>`;
}); });
container.innerHTML = html; container.innerHTML = html;
@@ -120,15 +111,6 @@ function renderWalletListSettings() {
}); });
}); });
container.querySelectorAll(".btn-show-phrase").forEach((btn) => {
btn.addEventListener("click", () => {
const idx = parseInt(btn.dataset.idx, 10);
// No pushCurrentView() here: showPhrase.show() refuses
// non-HD wallets and pushes only when it navigates.
showPhrase.show(idx);
});
});
// Inline rename on click // Inline rename on click
container.querySelectorAll(".settings-wallet-name").forEach((span) => { container.querySelectorAll(".settings-wallet-name").forEach((span) => {
span.addEventListener("click", () => { span.addEventListener("click", () => {
@@ -209,7 +191,6 @@ function renderSiteLists() {
function init(ctx) { function init(ctx) {
deleteWallet.init(ctx); deleteWallet.init(ctx);
showPhrase.init();
$("btn-save-rpc").addEventListener("click", async () => { $("btn-save-rpc").addEventListener("click", async () => {
const url = $("settings-rpc").value.trim(); const url = $("settings-rpc").value.trim();

View File

@@ -1,154 +0,0 @@
// Recovery phrase display for HD wallets.
//
// The phrase is the secret that owns every address in the wallet, so it is
// handled under four rules:
//
// 1. Only an HD wallet reaches this screen (walletHasRecoveryPhrase).
// 2. Nothing is decrypted, and nothing is written into the DOM, until
// decryptWithPassword has accepted the password.
// 3. Leaving the screen by any path wipes it, via the onViewLeave hook,
// and a decrypt still in flight when that happens is discarded
// instead of written (revealGeneration).
// 4. The phrase never reaches the logger. This module deliberately does
// not import src/shared/log.js, and the failed-decrypt path reports a
// fixed sentence rather than the caught error.
//
// The phrase is also never assigned to `state`, so it cannot be persisted
// to extension storage, and "show-phrase" is excluded from RESTORABLE_VIEWS
// so the popup can never reopen onto it.
const {
$,
showView,
showFlash,
flashCopyFeedback,
goBack,
onViewLeave,
pushCurrentView,
} = require("./helpers");
const { state } = require("../../shared/state");
const { decryptWithPassword } = require("../../shared/vault");
const { walletHasRecoveryPhrase } = require("../../shared/wallet");
const VIEW = "show-phrase";
let walletIndex = null;
// Bumped by every clear(), which is what leaving the screen runs. reveal()
// captures it before awaiting the decrypt and refuses to touch the DOM if
// it has moved: a decrypt still in flight when the screen is left would
// otherwise write the phrase *after* the wipe, with nothing scheduled to
// wipe it again, leaving it in the hidden view for the life of the popup.
let revealGeneration = 0;
// True only if the reveal that captured `generation` is still the live one:
// the screen has not been left, cleared, or re-entered for another wallet
// since it started.
function isCurrentReveal(generation) {
return (
generation === revealGeneration &&
walletIndex !== null &&
state.currentView === VIEW
);
}
function fail(message) {
$("show-phrase-flash").textContent = message;
$("show-phrase-flash").style.visibility = "visible";
}
// Wipe every trace of the phrase and drop the wallet selection. Safe to
// call when nothing was ever revealed, and safe to call twice.
function clear() {
walletIndex = null;
revealGeneration += 1;
$("show-phrase-value").textContent = "";
$("show-phrase-password").value = "";
$("show-phrase-result").classList.add("hidden");
$("show-phrase-password-section").classList.remove("hidden");
$("show-phrase-flash").textContent = "";
$("show-phrase-flash").style.visibility = "hidden";
}
function show(walletIdx) {
const wallet = state.wallets[walletIdx];
if (!walletHasRecoveryPhrase(wallet)) {
showFlash("This wallet does not have a recovery phrase.");
return;
}
clear();
walletIndex = walletIdx;
$("show-phrase-wallet-name").textContent =
wallet.name || "Wallet " + (walletIdx + 1);
// Pushed here rather than by the caller: this function can return
// without navigating, and a push that happened anyway would leave an
// entry on the stack that no screen transition matches.
pushCurrentView();
showView(VIEW);
}
async function reveal() {
const password = $("show-phrase-password").value;
if (!password) {
fail("Please enter your password.");
return;
}
if (walletIndex === null) {
fail("No wallet is selected.");
return;
}
const wallet = state.wallets[walletIndex];
if (!walletHasRecoveryPhrase(wallet)) {
fail("This wallet does not have a recovery phrase.");
return;
}
const btn = $("btn-show-phrase-reveal");
btn.disabled = true;
btn.classList.add("text-muted");
const generation = revealGeneration;
try {
const phrase = await decryptWithPassword(
wallet.encryptedSecret,
password,
);
// The only suspension point in this view, and the only place a
// secret is written: if the screen was left while the decrypt ran,
// the wipe has already happened and this write must not land.
if (!isCurrentReveal(generation)) return;
$("show-phrase-password").value = "";
$("show-phrase-password-section").classList.add("hidden");
$("show-phrase-value").textContent = phrase;
$("show-phrase-result").classList.remove("hidden");
$("show-phrase-flash").textContent = "";
$("show-phrase-flash").style.visibility = "hidden";
} catch {
if (!isCurrentReveal(generation)) return;
// Deliberately not the caught error: the message is fixed so that
// nothing derived from the ciphertext or the attempt can surface.
fail("That password is not correct. Please try again.");
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");
}
}
function init() {
onViewLeave(VIEW, clear);
$("btn-show-phrase-back").addEventListener("click", () => {
goBack();
});
$("btn-show-phrase-reveal").addEventListener("click", reveal);
$("show-phrase-value").addEventListener("click", () => {
const phrase = $("show-phrase-value").textContent;
if (!phrase) return;
navigator.clipboard.writeText(phrase);
showFlash("Copied!");
flashCopyFeedback($("show-phrase-value"));
});
}
module.exports = { init, show };

View File

@@ -16,60 +16,8 @@ function generateMnemonic() {
return m.phrase; return m.phrase;
} }
// Every extended key (xprv or xpub) entering the app goes through this.
//
// ethers' HDNodeWallet.fromExtendedKey does NOT verify the base58 checksum
// when the decoded payload is the usual 82 bytes, which is exactly the case
// the checksum exists to catch: a key with a one-character typo parses into a
// *different* wallet instead of being rejected. Re-encoding the parsed node
// reproduces a well-formed key byte for byte, checksum included, so comparing
// the round trip against the input rejects any altered character. Measured by
// the sweep in tests/wallet.test.js over every single-character substitution
// of the BIP-32 vector 1 master key: 199 parse without the round-trip
// comparison, 0 with it.
//
// Returns the parsed node, or null if the key is not a well-formed extended
// key. Callers turn null into a user-facing error; none of them may fall back
// to fromExtendedKey directly.
function parseExtendedKey(key) {
if (typeof key !== "string") return null;
try {
const node = HDNodeWallet.fromExtendedKey(key);
return node.extendedKey === key ? node : null;
} catch {
return null;
}
}
// A master key is at depth 0. Only from there is BIP44_ETH_PATH the absolute
// path it names; deriving it under an account-level or child key yields
// addresses that correspond to nothing the user holds.
const MASTER_DEPTH = 0;
// Parse an extended private key that the BIP-44 Ethereum account path can be
// derived from, or throw. Both callers derive BIP44_ETH_PATH from the result.
function masterXprvOrThrow(key) {
const node = parseExtendedKey(key);
if (!node) {
throw new Error("Not a valid extended private key (xprv).");
}
if (!node.privateKey) {
throw new Error("Not an extended private key (xprv).");
}
if (node.depth !== MASTER_DEPTH) {
throw new Error(
"Not a master extended private key (xprv): an account-level or " +
"child key cannot be imported.",
);
}
return node;
}
function deriveAddressFromXpub(xpub, index) { function deriveAddressFromXpub(xpub, index) {
const node = parseExtendedKey(xpub); const node = HDNodeWallet.fromExtendedKey(xpub);
if (!node) {
throw new Error("Not a valid extended key.");
}
return node.deriveChild(index).address; return node.deriveChild(index).address;
} }
@@ -81,28 +29,23 @@ function hdWalletFromMnemonic(mnemonic) {
} }
function hdWalletFromXprv(xprv) { function hdWalletFromXprv(xprv) {
// BIP44_ETH_PATH is absolute ("m/..."), which ethers will only derive from const root = HDNodeWallet.fromExtendedKey(xprv);
// a depth-0 node. The relative form this used to derive would have been if (!root.privateKey) {
// applied *beneath* an account-level key instead of being refused. throw new Error("Not an extended private key (xprv).");
const node = masterXprvOrThrow(xprv).derivePath(BIP44_ETH_PATH); }
const node = root.derivePath("44'/60'/0'/0");
const xpub = node.neuter().extendedKey; const xpub = node.neuter().extendedKey;
const firstAddress = node.deriveChild(0).address; const firstAddress = node.deriveChild(0).address;
return { xpub, firstAddress }; return { xpub, firstAddress };
} }
// Well-formed extended private key. Says nothing about depth: the import view
// reports a non-master key separately, since "check it for a typo" is the
// wrong advice for a key the user copied correctly.
function isValidXprv(key) { function isValidXprv(key) {
const node = parseExtendedKey(key); try {
return !!(node && node.privateKey); const node = HDNodeWallet.fromExtendedKey(key);
return !!node.privateKey;
} catch {
return false;
} }
// Whether an extended key is a master key, i.e. the one BIP44_ETH_PATH can be
// derived from. False for anything parseExtendedKey rejects.
function isMasterExtendedKey(key) {
const node = parseExtendedKey(key);
return !!node && node.depth === MASTER_DEPTH;
} }
function addressFromPrivateKey(key) { function addressFromPrivateKey(key) {
@@ -120,8 +63,8 @@ function getSignerForAddress(walletData, addrIndex, decryptedSecret) {
return node.deriveChild(addrIndex); return node.deriveChild(addrIndex);
} }
if (walletData.type === "xprv") { if (walletData.type === "xprv") {
const node = const root = HDNodeWallet.fromExtendedKey(decryptedSecret);
masterXprvOrThrow(decryptedSecret).derivePath(BIP44_ETH_PATH); const node = root.derivePath("44'/60'/0'/0");
return node.deriveChild(addrIndex); return node.deriveChild(addrIndex);
} }
return new Wallet(decryptedSecret); return new Wallet(decryptedSecret);
@@ -131,24 +74,13 @@ function isValidMnemonic(mnemonic) {
return Mnemonic.isValidMnemonic(mnemonic); return Mnemonic.isValidMnemonic(mnemonic);
} }
// Only an HD wallet has a recovery phrase. A "key" wallet holds a bare
// private key and an "xprv" wallet an extended private key; neither can be
// turned back into words, so neither may ever be offered the phrase display.
// Written as an allowlist on purpose: a wallet type added later is excluded
// until someone decides otherwise.
function walletHasRecoveryPhrase(walletData) {
return !!walletData && walletData.type === "hd";
}
module.exports = { module.exports = {
generateMnemonic, generateMnemonic,
deriveAddressFromXpub, deriveAddressFromXpub,
hdWalletFromMnemonic, hdWalletFromMnemonic,
hdWalletFromXprv, hdWalletFromXprv,
isValidXprv, isValidXprv,
isMasterExtendedKey,
addressFromPrivateKey, addressFromPrivateKey,
getSignerForAddress, getSignerForAddress,
isValidMnemonic, isValidMnemonic,
walletHasRecoveryPhrase,
}; };

View File

@@ -255,11 +255,6 @@ async function openPopup(ctx, popupUrl) {
// Full wallet creation through the real UI: BIP-39 generation, libsodium // Full wallet creation through the real UI: BIP-39 generation, libsodium
// vault encryption and extension storage persistence, for real. // vault encryption and extension storage persistence, for real.
//
// Returns the recovery phrase it generated. Tests that assert on a secret
// need the real value — checking for "some 12 words" would pass against the
// wrong wallet's phrase, and checking for nothing at all would pass against
// a screen that shows the phrase it was supposed to hide.
async function createWallet(page) { async function createWallet(page) {
await page.click("#btn-welcome-add"); await page.click("#btn-welcome-add");
await visible(page, "#view-add-wallet"); await visible(page, "#view-add-wallet");
@@ -268,12 +263,10 @@ async function createWallet(page) {
const el = document.getElementById("wallet-mnemonic"); const el = document.getElementById("wallet-mnemonic");
return el && el.value.trim().split(/\s+/).length >= 12; return el && el.value.trim().split(/\s+/).length >= 12;
}); });
const phrase = (await page.inputValue("#wallet-mnemonic")).trim();
await page.fill("#add-wallet-password", PASSWORD); await page.fill("#add-wallet-password", PASSWORD);
await page.fill("#add-wallet-password-confirm", PASSWORD); await page.fill("#add-wallet-password-confirm", PASSWORD);
await page.click("#btn-add-wallet-confirm"); await page.click("#btn-add-wallet-confirm");
await visible(page, "#view-main", 60000); await visible(page, "#view-main", 60000);
return phrase;
} }
// Reach the address detail screen from wherever the popup restored to. // Reach the address detail screen from wherever the popup restored to.
@@ -288,7 +281,6 @@ async function openAddressDetail(page) {
} }
module.exports = { module.exports = {
PASSWORD,
createWallet, createWallet,
launch, launch,
openAddressDetail, openAddressDetail,

View File

@@ -10,7 +10,6 @@
"use strict"; "use strict";
const { const {
PASSWORD,
createWallet, createWallet,
launch, launch,
openAddressDetail, openAddressDetail,
@@ -35,10 +34,6 @@ function assert(cond, message) {
if (!cond) throw new Error(message); if (!cond) throw new Error(message);
} }
function sleep(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}
function withTimeout(promise, name) { function withTimeout(promise, name) {
let timer; let timer;
const timeout = new Promise((_, reject) => { const timeout = new Promise((_, reject) => {
@@ -61,11 +56,7 @@ test("popup loads and reaches the welcome view", async (env) => {
}); });
test("wallet creation through the UI reaches the main view", async (env) => { test("wallet creation through the UI reaches the main view", async (env) => {
env.phrase = await createWallet(env.page); await createWallet(env.page);
assert(
env.phrase.split(/\s+/).length >= 12,
"wallet creation did not yield a recovery phrase",
);
const addrCount = await env.page const addrCount = await env.page
.locator("#wallet-list .btn-addr-info") .locator("#wallet-list .btn-addr-info")
.count(); .count();
@@ -126,256 +117,6 @@ test("transaction detail renders an ERC-20 transfer (#151)", async (env) => {
assert(dots > 0, "token contract row rendered without its colour dot"); assert(dots > 0, "token contract row rendered without its colour dot");
}); });
// -------------------------------------------- recovery phrase (#161)
// The gear toggles, so pressing it while Settings is already up leaves it.
async function openSettings(page) {
if (!(await page.isVisible("#view-settings"))) {
await page.click("#btn-settings");
}
await visible(page, "#view-settings");
}
// Everything the recovery phrase screen is holding, read straight out of
// 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
// merely off-screen.
async function phraseScreenState(page) {
return page.evaluate(() => ({
value: document.getElementById("show-phrase-value").textContent,
error: document.getElementById("show-phrase-flash").textContent,
html: document.getElementById("view-show-phrase").innerHTML,
resultHidden: document
.getElementById("show-phrase-result")
.classList.contains("hidden"),
viewHidden: document
.getElementById("view-show-phrase")
.classList.contains("hidden"),
}));
}
async function openPhraseScreen(page) {
await openSettings(page);
await page.click("#settings-wallet-list .btn-show-phrase");
await visible(page, "#view-show-phrase");
}
async function revealPhrase(page) {
await page.fill("#show-phrase-password", PASSWORD);
await page.click("#btn-show-phrase-reveal");
await visible(page, "#show-phrase-result", 60000);
}
function assertWiped(st, phrase, where) {
assert(st.value === "", "phrase still in the DOM " + where);
assert(st.resultHidden, "result section still shown " + where);
assert(
!st.html.includes(phrase),
"the recovery phrase is still somewhere in the screen markup " + where,
);
}
test("only an HD wallet is offered the recovery phrase action (#161)", async (env) => {
await openSettings(env.page);
const offered = await env.page
.locator("#settings-wallet-list .btn-show-phrase")
.count();
const wallets = await env.page
.locator("#settings-wallet-list .btn-delete-wallet")
.count();
assert(wallets === 1, "expected exactly one wallet row, got " + wallets);
assert(
offered === 1,
"the HD wallet was not offered the recovery phrase action",
);
});
// 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.
// The key is generated here rather than committed — the repo holds no
// private keys, test ones included.
test("a key wallet is not offered the recovery phrase action (#161)", async (env) => {
const { Wallet } = require("ethers");
await openSettings(env.page);
await env.page.click("#btn-main-add-wallet");
await visible(env.page, "#view-add-wallet");
await env.page.click("#tab-privkey");
await env.page.fill(
"#import-private-key",
Wallet.createRandom().privateKey,
);
await env.page.fill("#add-wallet-password", PASSWORD);
await env.page.fill("#add-wallet-password-confirm", PASSWORD);
await env.page.click("#btn-add-wallet-confirm");
await visible(env.page, "#view-main", 60000);
await openSettings(env.page);
const wallets = await env.page
.locator("#settings-wallet-list .btn-delete-wallet")
.count();
const offered = await env.page
.locator("#settings-wallet-list .btn-show-phrase")
.count();
assert(wallets === 2, "expected two wallet rows, got " + wallets);
assert(
offered === 1,
"the key wallet was offered the recovery phrase action",
);
});
test("the recovery phrase screen holds nothing before the password (#161)", async (env) => {
await openPhraseScreen(env.page);
const st = await phraseScreenState(env.page);
assertWiped(st, env.phrase, "before any password was entered");
const passwordShown = await env.page.isVisible(
"#show-phrase-password-section",
);
assert(passwordShown, "the password prompt is not shown");
});
test("a wrong password reveals nothing (#161)", async (env) => {
await env.page.fill("#show-phrase-password", "not-the-password");
await env.page.click("#btn-show-phrase-reveal");
await env.page.waitForFunction(
() =>
document.getElementById("show-phrase-flash").textContent.length > 0,
null,
{ timeout: 60000 },
);
const st = await phraseScreenState(env.page);
assertWiped(st, env.phrase, "after a wrong password");
assert(
/^[A-Z].*\.$/.test(st.error.trim()),
"the wrong-password error is not a full sentence: " +
JSON.stringify(st.error),
);
});
test("the correct password reveals the full phrase, and nothing logs it (#161)", async (env) => {
const console_ = [];
const listener = (msg) => console_.push(msg.text());
env.page.on("console", listener);
try {
await revealPhrase(env.page);
const st = await phraseScreenState(env.page);
assert(
st.value === env.phrase,
"the displayed phrase is not the wallet's phrase, verbatim",
);
const promptShown = await env.page.isVisible(
"#show-phrase-password-section",
);
assert(!promptShown, "the password prompt is still shown after unlock");
// Full Identifiers Policy: shown whole, and copyable.
const title = await env.page.getAttribute(
"#show-phrase-value",
"title",
);
assert(title === "Click to copy", "the phrase is not click-to-copy");
const leaked = console_.filter((line) => line.includes(env.phrase));
assert(
leaked.length === 0,
"the recovery phrase reached the console: " +
JSON.stringify(leaked),
);
} finally {
env.page.off("console", listener);
}
});
test('"Back" wipes the revealed phrase (#161)', async (env) => {
await env.page.click("#btn-show-phrase-back");
await visible(env.page, "#view-settings");
const st = await phraseScreenState(env.page);
assert(st.viewHidden, "the recovery phrase screen is still on top");
assertWiped(st, env.phrase, "after Back");
});
// The settings gear leaves the screen without touching its Back button. A
// clear wired only to Back would pass the test above and leak here.
test("leaving by the settings gear wipes it too (#161)", async (env) => {
await openPhraseScreen(env.page);
await revealPhrase(env.page);
await env.page.click("#btn-settings");
await visible(env.page, "#view-settings");
const st = await phraseScreenState(env.page);
assertWiped(st, env.phrase, "after leaving via the settings gear");
});
// The same leave, but taken while the decrypt is still running. Both
// clicks are dispatched inside one page task on purpose: "Reveal" runs its
// handler up to the await, the gear then runs the leave — and the wipe with
// it — to completion, and the decrypt's continuation resumes afterwards.
// Without a liveness check that continuation writes the phrase into the
// hidden screen after the wipe, and nothing is left to wipe it again.
//
// A human cannot produce this interleaving by hand once libsodium's wasm is
// warm, because crypto_pwhash is synchronous and the only suspension point
// is a microtask; the window a user can actually hit is a still-pending
// sodium.ready on the first vault use of a page load. Forcing it here is
// the only way to test the guard deterministically.
test("leaving while the decrypt is in flight reveals nothing (#161)", async (env) => {
await openPhraseScreen(env.page);
await env.page.fill("#show-phrase-password", PASSWORD);
await env.page.evaluate(() => {
document.getElementById("btn-show-phrase-reveal").click();
document.getElementById("btn-settings").click();
});
await visible(env.page, "#view-settings");
// The Reveal button is disabled for exactly the duration of the
// decrypt and re-enabled in the same continuation that would have
// written the phrase, so waiting for it to come back is a precise
// "the decrypt has settled and its handler has finished" signal
// rather than a guess at a duration.
await env.page.waitForFunction(
() => !document.getElementById("btn-show-phrase-reveal").disabled,
null,
{ timeout: 60000 },
);
await sleep(2000);
const st = await phraseScreenState(env.page);
// 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
// same line read from a build without the guard is what this test
// exists to prevent.
console.log(
"# probe: len=" +
st.value.length +
" equalsPhrase=" +
(st.value === env.phrase) +
" resultHidden=" +
st.resultHidden +
" viewHidden=" +
st.viewHidden,
);
assert(st.viewHidden, "the recovery phrase screen is still on top");
assertWiped(st, env.phrase, "after leaving mid-decrypt");
});
// Closing and reopening the page rather than reloading it: that is what
// the toolbar popup actually does, and the persisted currentView is
// "show-phrase" at the moment it happens, which is precisely the state
// RESTORABLE_VIEWS has to refuse.
test("reopening the popup never lands on the phrase screen (#161)", async (env) => {
await openPhraseScreen(env.page);
await revealPhrase(env.page);
await env.page.close();
env.page = await openPopup(env.ctx, env.popupUrl);
await visible(env.page, "#view-main");
const st = await phraseScreenState(env.page);
assert(st.viewHidden, "the popup reopened onto the recovery phrase screen");
assertWiped(st, env.phrase, "after reopening the popup");
});
// ---------------------------------------------------------------- runner // ---------------------------------------------------------------- runner
async function main() { async function main() {
@@ -413,9 +154,6 @@ async function main() {
popupUrl: session.popupUrl, popupUrl: session.popupUrl,
routeOpts, routeOpts,
page: null, page: null,
// The recovery phrase of the wallet created in test 2, so later
// tests can assert on the real secret rather than its shape.
phrase: null,
}; };
// Attribution of collected errors is total. session.errors has no // Attribution of collected errors is total. session.errors has no

View File

@@ -1,111 +0,0 @@
// Tests for the UTC Timestamps setting.
//
// The checkbox was moved out of the Token Spam Protection well and into the
// Display well next to the theme selector. It is wired by id through the $()
// helper, so the move cannot break the handler — but nothing in the suite said
// so. These tests pin both halves down: the markup lives in Display and
// nowhere else, and the value still round-trips through storage.
const fs = require("fs");
const path = require("path");
const POPUP_HTML = fs.readFileSync(
path.join(__dirname, "..", "src", "popup", "index.html"),
"utf8",
);
// The body of one `<div class="bg-well ...">` well, selected by its heading.
function wellWithHeading(html, heading) {
const headingIndex = html.indexOf(
'<h3 class="font-bold mb-1">' + heading + "</h3>",
);
expect(headingIndex).toBeGreaterThan(-1);
const start = html.lastIndexOf('<div class="bg-well', headingIndex);
const end = html.indexOf('<div class="bg-well', headingIndex);
return html.slice(start, end === -1 ? html.length : end);
}
describe("the UTC Timestamps checkbox placement", () => {
test("the checkbox appears exactly once in the popup markup", () => {
const matches = POPUP_HTML.match(/id="settings-utc-timestamps"/g);
expect(matches).toHaveLength(1);
});
test("it renders in the Display well, alongside the theme selector", () => {
const display = wellWithHeading(POPUP_HTML, "Display");
expect(display).toContain('id="settings-utc-timestamps"');
expect(display).toContain('id="settings-theme"');
});
test("it does not render in the Token Spam Protection well", () => {
const spam = wellWithHeading(POPUP_HTML, "Token Spam Protection");
expect(spam).not.toContain('id="settings-utc-timestamps"');
// The filters that do belong there are untouched.
expect(spam).toContain('id="settings-hide-low-holders"');
expect(spam).toContain('id="settings-hide-fraud-contracts"');
expect(spam).toContain('id="settings-hide-dust"');
expect(spam).toContain('id="settings-dust-threshold"');
});
});
describe("the UTC Timestamps setting round-trips through storage", () => {
let store;
function loadStateModule() {
store = {};
global.chrome = {
storage: {
local: {
get: async (key) =>
key in store ? { [key]: store[key] } : {},
set: async (obj) => Object.assign(store, obj),
},
},
};
jest.resetModules();
return require("../src/shared/state");
}
afterEach(() => {
delete global.chrome;
});
test("defaults to off with nothing persisted", async () => {
const { state, loadState } = loadStateModule();
await loadState();
expect(state.utcTimestamps).toBe(false);
});
test("an enabled checkbox is persisted and read back", async () => {
const first = loadStateModule();
// What the change handler in views/settings.js does.
first.state.utcTimestamps = true;
await first.saveState();
expect(store.autistmask.utcTimestamps).toBe(true);
// A fresh popup load sees it.
jest.resetModules();
const second = require("../src/shared/state");
expect(second.state.utcTimestamps).toBe(false);
await second.loadState();
expect(second.state.utcTimestamps).toBe(true);
});
test("turning it back off is persisted too", async () => {
const { state, saveState, loadState } = loadStateModule();
state.utcTimestamps = true;
await saveState();
state.utcTimestamps = false;
await saveState();
state.utcTimestamps = true;
await loadState();
expect(state.utcTimestamps).toBe(false);
});
});

View File

@@ -1,94 +0,0 @@
// Tests for the recovery phrase display (issue #161).
//
// These cover the parts that do not need a DOM: which wallet types may be
// offered the action at all, the exclusion of the screen from the set of
// views the popup may reopen onto, and the absence of any path from this
// module to the logger. The DOM behaviour it guards — nothing rendered
// before the password is accepted, a wrong password revealing nothing, and
// the wipe on leaving — is driven against the real popup in a real browser
// by tests/e2e/run.js, which is where every other view behaviour is tested.
const fs = require("fs");
const path = require("path");
const { walletHasRecoveryPhrase } = require("../src/shared/wallet");
const { RESTORABLE_VIEWS } = require("../src/popup/restorableViews");
const SHOW_PHRASE_VIEW = "show-phrase";
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
function loadHelpers() {
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
return require("../src/popup/views/helpers");
}
describe("which wallets have a recovery phrase", () => {
test("an HD wallet does", () => {
expect(walletHasRecoveryPhrase({ type: "hd" })).toBe(true);
});
// A key wallet holds a bare private key and an xprv wallet an extended
// private key. Neither can be turned back into words, so neither may be
// offered the action.
test("a key wallet does not", () => {
expect(walletHasRecoveryPhrase({ type: "key" })).toBe(false);
});
test("an xprv wallet does not", () => {
expect(walletHasRecoveryPhrase({ type: "xprv" })).toBe(false);
});
test("an unknown or missing wallet type does not", () => {
expect(walletHasRecoveryPhrase({ type: "something-new" })).toBe(false);
expect(walletHasRecoveryPhrase({})).toBe(false);
expect(walletHasRecoveryPhrase(undefined)).toBe(false);
});
});
describe("views the popup may reopen onto", () => {
// Restoring onto a secret screen would put the phrase on screen with no
// password prompt in front of it, on a popup the user may have reopened
// by accident.
test("the recovery phrase screen is not restorable", () => {
expect(RESTORABLE_VIEWS.has(SHOW_PHRASE_VIEW)).toBe(false);
});
test("the private key export screen is not restorable either", () => {
expect(RESTORABLE_VIEWS.has("export-privkey")).toBe(false);
});
test("the recovery phrase screen is still a registered view", () => {
const { VIEWS } = loadHelpers();
expect(VIEWS).toContain(SHOW_PHRASE_VIEW);
});
// Guards the other direction: a restorable name that is not a real view
// would leave restoreView() showing nothing at all.
test("every restorable view is a registered view", () => {
const { VIEWS } = loadHelpers();
for (const view of RESTORABLE_VIEWS) {
expect(VIEWS).toContain(view);
}
});
});
describe("the phrase cannot reach the logger", () => {
const source = fs.readFileSync(
path.join(__dirname, "..", "src", "popup", "views", "showPhrase.js"),
"utf8",
);
// The decrypted phrase only ever lives in a local and in the DOM node
// that displays it. The module has no logger to hand it to, and this
// pins that: src/shared/log.js writes to the console, and a console
// record of a recovery phrase outlives the popup.
test("the view does not import src/shared/log.js", () => {
expect(source).not.toMatch(/require\(["'][^"']*shared\/log["']\)/);
});
test("the view calls no logger method", () => {
expect(source).not.toMatch(/\blog\.(debugf|infof|warnf|errorf)\b/);
});
});

View File

@@ -160,31 +160,6 @@ function masterXprv(phrase, passphrase = "") {
).extendedKey; ).extendedKey;
} }
// The account-level (depth-3) extended private key m/44'/60'/0' for a phrase.
// A normal thing for a user to hold, and not something the import flow can
// derive the BIP-44 account path from.
function accountXprv(phrase) {
return HDNodeWallet.fromSeed(
Mnemonic.fromPhrase(phrase, "").computeSeed(),
).derivePath("m/44'/60'/0'").extendedKey;
}
// Every single-character substitution of `key`, using base58 characters that
// are not the original. Base58 has no visually ambiguous characters, so each
// of these is a plausible typo rather than a contrived string.
const TYPO_CHARS = ["a", "b", "2", "Z"];
function singleCharacterTypos(key) {
const out = [];
for (let i = 0; i < key.length; i++) {
for (const c of TYPO_CHARS) {
if (c === key[i]) continue;
out.push(key.slice(0, i) + c + key.slice(i + 1));
}
}
return out;
}
describe("hdWalletFromMnemonic", () => { describe("hdWalletFromMnemonic", () => {
test("first address matches the published vector for m/44'/60'/0'/0/0", () => { test("first address matches the published vector for m/44'/60'/0'/0/0", () => {
expect(wallet.hdWalletFromMnemonic(VECTOR_PHRASE).firstAddress).toBe( expect(wallet.hdWalletFromMnemonic(VECTOR_PHRASE).firstAddress).toBe(
@@ -324,7 +299,19 @@ describe("isValidXprv", () => {
expect(wallet.isValidXprv(xpub)).toBe(false); expect(wallet.isValidXprv(xpub)).toBe(false);
}); });
test("rejects an extended key with a one-character typo", () => { // Skipped: this asserts the correct behaviour, which the code does not
// currently have. isValidXprv gates the paste-your-extended-private-key
// import in src/popup/views/addWallet.js:215, and it accepts a key with a
// one-character typo: ethers' HDNodeWallet.fromExtendedKey skips base58
// checksum verification whenever the decoded payload is the usual 82
// bytes, which is the whole point of that checksum. Measured on this
// vector: changing any one of the last 14 characters passes validation,
// and for 9 of those 14 positions the import silently yields a *different*
// wallet (e.g. 0x3F334f0a356d6B46B1d70B590E7437D77100d28D instead of
// 0x022b971dFF0C43305e691DEd7a14367AF19D6407) with no error shown.
// Tracked as https://git.eeqj.de/sneak/AutistMask/issues/210; out of scope
// here, which is tests only. Unskip when it is fixed.
test.skip("rejects an extended key with a one-character typo", () => {
const index = BIP32_VECTOR_1_XPRV.length - 8; const index = BIP32_VECTOR_1_XPRV.length - 8;
const typo = const typo =
BIP32_VECTOR_1_XPRV.slice(0, index) + BIP32_VECTOR_1_XPRV.slice(0, index) +
@@ -333,125 +320,6 @@ describe("isValidXprv", () => {
expect(wallet.isValidXprv(typo)).toBe(false); expect(wallet.isValidXprv(typo)).toBe(false);
}); });
// The base58 checksum exists to make a mistyped key impossible to use, and
// ethers does not enforce it: HDNodeWallet.fromExtendedKey skips checksum
// verification whenever the decoded payload is the usual 82 bytes, which
// is precisely the case it is there to catch. A typo anywhere in the key
// must be refused, not silently turned into someone else's wallet.
test("no single-character typo anywhere in the key is accepted", () => {
const accepted = singleCharacterTypos(BIP32_VECTOR_1_XPRV).filter(
(typo) => wallet.isValidXprv(typo),
);
expect(accepted).toEqual([]);
});
test("a typo never yields a wallet, let alone a different one", () => {
const correct = wallet.hdWalletFromXprv(BIP32_VECTOR_1_XPRV);
const derived = [];
for (const typo of singleCharacterTypos(BIP32_VECTOR_1_XPRV)) {
try {
derived.push(wallet.hdWalletFromXprv(typo).firstAddress);
} catch {
// Rejected, which is the required behaviour.
}
}
expect(derived).toEqual([]);
expect(correct.firstAddress).toBe(
"0x022b971dFF0C43305e691DEd7a14367AF19D6407",
);
});
});
describe("extended key depth", () => {
// hdWalletFromXprv derives the BIP-44 Ethereum account path from the key
// it is given. That is only the path it names when the key is the master
// key. Under an account-level key the same derivation lands at
// m/44'/60'/0'/44'/60'/0'/0, whose addresses correspond to nothing the
// user holds, so a non-master key is refused rather than derived from.
test("a master key is a master key", () => {
expect(wallet.isMasterExtendedKey(masterXprv(VECTOR_PHRASE))).toBe(
true,
);
expect(wallet.isMasterExtendedKey(BIP32_VECTOR_1_XPRV)).toBe(true);
});
test("an account-level key is not a master key", () => {
expect(wallet.isMasterExtendedKey(accountXprv(VECTOR_PHRASE))).toBe(
false,
);
});
test("a derived xpub is not a master key", () => {
expect(
wallet.isMasterExtendedKey(
wallet.hdWalletFromMnemonic(VECTOR_PHRASE).xpub,
),
).toBe(false);
});
test("a mistyped key is not a master key either", () => {
expect(wallet.isMasterExtendedKey(BIP32_VECTOR_1_XPRV + "a")).toBe(
false,
);
});
test("hdWalletFromXprv rejects an account-level key", () => {
expect(() =>
wallet.hdWalletFromXprv(accountXprv(VECTOR_PHRASE)),
).toThrow(/master/i);
});
test("getSignerForAddress rejects an account-level key", () => {
expect(() =>
wallet.getSignerForAddress(
{ type: "xprv" },
0,
accountXprv(VECTOR_PHRASE),
),
).toThrow(/master/i);
});
test("the account-level key is well-formed, so only depth rejects it", () => {
expect(wallet.isValidXprv(accountXprv(VECTOR_PHRASE))).toBe(true);
});
test("a master key still imports and derives the published addresses", () => {
const { xpub, firstAddress } = wallet.hdWalletFromXprv(
masterXprv(VECTOR_PHRASE),
);
expect(firstAddress).toBe(VECTOR_ADDRESSES[0]);
expect(
[0, 1, 2].map((i) => wallet.deriveAddressFromXpub(xpub, i)),
).toEqual(VECTOR_ADDRESSES);
});
});
describe("deriveAddressFromXpub checksum enforcement", () => {
// The xpub path shares the hole: fromExtendedKey accepts a mistyped xpub
// just as readily, and deriveAddressFromXpub would hand back addresses
// from a different tree.
const { xpub } = wallet.hdWalletFromMnemonic(VECTOR_PHRASE);
test("the correct xpub still derives the published addresses", () => {
expect(wallet.deriveAddressFromXpub(xpub, 0)).toBe(VECTOR_ADDRESSES[0]);
});
test("no single-character typo anywhere in an xpub is accepted", () => {
const derived = [];
for (const typo of singleCharacterTypos(xpub)) {
try {
derived.push(wallet.deriveAddressFromXpub(typo, 0));
} catch {
// Rejected, which is the required behaviour.
}
}
expect(derived).toEqual([]);
});
}); });
describe("isValidMnemonic", () => { describe("isValidMnemonic", () => {