Compare commits

..
1 Commits
Author SHA1 Message Date
clawbot 2a918bf6bf fix: name a tracked or explorer-known token instead of "Unknown token" (closes #323)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
The approval and transaction-status screens read a token's scale from the
bundled list, the tokens the user tracks, then the block explorer, but read
its symbol from the bundled list alone. A token the user added by hand was
scaled correctly yet labelled "Unknown token", and a non-bundled ERC-20 was
carried onto the wait screen as ETH.

resolveTokenSymbol() now draws the symbol through the same sources and
precedence as the scale, and the ERC-20 and Uniswap swap lines both use it. A
tracked or explorer-reported name stays subject to the spoof rule, so it
cannot claim a bundled or native ticker.

Folds in #354.

Model: opus-4-8
2026-09-21 07:48:21 +00:00
22 changed files with 50 additions and 970 deletions
+1 -3
View File
@@ -64,9 +64,7 @@ release/SHA256SUMS
``` ```
Nothing is published by this. Tagging, CRX packing and any upload are Nothing is published by this. Tagging, CRX packing and any upload are
outward-facing acts and are the owner's alone. The full procedure that turns a outward-facing acts and are the owner's alone.
green `main` into a tagged, packaged release — the order of steps, who performs
each, and how to check it worked — is in [docs/RELEASE.md](docs/RELEASE.md).
The archives are deterministic — entries sorted, timestamps fixed, compression The archives are deterministic — entries sorted, timestamps fixed, compression
level fixed — so two builds of one commit produce byte-identical files and the level fixed — so two builds of one commit produce byte-identical files and the
-88
View File
@@ -45,74 +45,6 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-09-21: The network fee a transaction can commit is bounded by the product
of the gas limit and the fee per gas, not by each field alone, and the
wallet's own send is bounded the same way
([#399](https://git.eeqj.de/sneak/AutistMask/issues/399)). The two per-field
ceilings in `src/shared/approvalVerify.js` were checked independently, so a
gas limit and a fee that were each under their own ceiling still multiplied to
thousands of ETH — a fee a gas-consuming contract really collects — while the
comment claimed the ceiling caught exactly that. `assertWithinCeilings` now
also refuses a transaction whose gas limit times its fee per gas
(`maxFeePerGas` for a type-2 transaction, `gasPrice` for a legacy or type-1
one) exceeds `MAX_TOTAL_FEE`, a new constant of 1 ETH beside the existing
ceilings, so both callers — where the dApp transaction is populated and where
the signed artifact is verified — reject it with a full sentence naming the
fee and the limit. The wallet's own send in `src/popup/views/confirmTx.js`
pinned no fee fields, so ethers filled them from whatever the configured node
answered with nothing bounding them; it now populates the transaction and runs
the same check before signing, showing the same error in the confirmation
screen's reserved errors box so nothing on screen moves. Deliberately out of
scope: comparing a supplied fee against the node's own suggested fee, which
the absolute bound already makes unnecessary for the balance-draining case. 1
ETH is a plain constant, one line to change; the owner may prefer another
figure.
- 2026-09-21: The test recovery phrase no longer survives in a release bundle,
and the committed-key guard matches by content
([#351](https://git.eeqj.de/sneak/AutistMask/issues/351)). `DEBUG_MNEMONIC` in
`src/shared/constants.js` is now behind the `__BUILD_DEBUG__` define, so a
release build folds the phrase to `null` and no emitted bundle carries it; it
used to survive as dead text because `module.exports` keeps the const alive.
`script/verify-build` now fails a release build if the phrase appears in any
emitted file, so the fold cannot silently regress. `tests/extensionId.test.js`
scans the content of every tracked file for a PEM private-key header instead
of matching filename extensions alone.
- 2026-09-21: A transaction response is honoured only for a transaction
approval, and the three remaining approval-settlement paths are pinned
([#262](https://git.eeqj.de/sneak/AutistMask/issues/262)). The liveness fix
the issue asks for — settle `4001` on release when the window it would be
retried in is gone — already landed with
[#271](https://git.eeqj.de/sneak/AutistMask/issues/271); this closes the rest.
`AUTISTMASK_TX_RESPONSE` now refuses any approval that is not a transaction
approval, so a reject no longer retires a sign or connection approval and a
signed artifact never runs the broadcast path against one, which before only
failed closed by throwing deeper in. Tests pin the site-connection port's
approve, reject and disconnect paths against a transaction approval
broadcasting behind them: each is declined and the dApp still receives its
broadcast result.
- 2026-09-21: `docs/RELEASE.md`, linked from `README.md`, states the release
procedure as a numbered list a newcomer can follow: confirm `main` is green in
CI, confirm the one version in the three files matches the intended tag,
`make package` from a clean checkout, verify `SHA256SUMS`, tag `vX.Y.Z`, then
distribute per browser. Each step names who performs it (owner-only steps
marked) and the check that it worked. The distribution step is written as
pending the owner's choice on
[#386](https://git.eeqj.de/sneak/AutistMask/issues/386), with the Firefox and
Chrome options named but none settled. Docs only
([#387](https://git.eeqj.de/sneak/AutistMask/issues/387)).
- 2026-09-21: Adding a second wallet no longer accepts a different password with
nothing saying it is a separate one
([#374](https://git.eeqj.de/sneak/AutistMask/issues/374)). Each wallet has its
own encrypted secret, so per-wallet passwords are by design; the add-wallet
screen said only "Choose a password". A note now appears on that screen when
the profile already holds a wallet, stating that each wallet has its own
password and this one need not match any already in use. It is shown only
then, since the first wallet has no other password to differ from, and it
stays consistent with the no-reset reality of
[#312](https://git.eeqj.de/sneak/AutistMask/issues/312) by promising no
recovery or reset.
- 2026-09-21: The dApp approval and transaction-status screens resolve a token's - 2026-09-21: The dApp approval and transaction-status screens resolve a token's
symbol from the bundled list, then the tokens the user tracks, then the block symbol from the bundled list, then the tokens the user tracks, then the block
explorer's report — the same sources and precedence the amount line already explorer's report — the same sources and precedence the amount line already
@@ -123,26 +55,6 @@ but the review is broader than any of them.
`Unknown token`, and a non-bundled ERC-20 is no longer carried onto the wait `Unknown token`, and a non-bundled ERC-20 is no longer carried onto the wait
screen as `ETH`. A tracked or explorer-reported name stays subject to the screen as `ETH`. A tracked or explorer-reported name stays subject to the
spoof rule, so resolving a symbol is not a new way to wear a known ticker. spoof rule, so resolving a symbol is not a new way to wear a known ticker.
- 2026-09-21: The debug/testnet banner no longer shows the internal view id to
the user in a release build
([#375](https://git.eeqj.de/sneak/AutistMask/issues/375)). The banner appended
the active view's id (e.g. `[TESTNET] (approve-tx)`), which is developer
vocabulary sitting directly above the approval screen's carefully worded
authorization text. The suffix is now gated on the compile-time `DEBUG`
constant rather than `isDebug()`, so it survives only in a debug build; a
testnet or the runtime debug toggle still raises the banner but without the
view id.
- 2026-09-21: The Confirm Delete button on the delete-wallet screen no longer
stays dead after a successful delete
([#335](https://git.eeqj.de/sneak/AutistMask/issues/335)). The password route
disabled the button before the decrypt and never re-enabled it, so a second
delete in the same popup session needed a reopen; the lost-password route
re-enabled its own button in its leave hook, so the two screens behaved
differently. Both now reset through the shared `finishDelete()`, the one path
both routes take, so they behave the same and the button is live for the next
delete.
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the - 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
wallet list was the reported case: the address shared one row with the wallet list was the reported case: the address shared one row with the
-87
View File
@@ -1,87 +0,0 @@
# Releasing AutistMask
This is the procedure that turns a green `main` into a tagged, packaged release.
It gathers into one place what is otherwise spread across the `Makefile` and
three `README.md` sections, so the person cutting a release does not have to
reconstruct the order from them.
There is one version, declared in three files (`package.json`,
`manifest/chrome.json`, `manifest/firefox.json`), and `make package` builds and
packages but publishes nothing. `make build` and `make package` can be run by
anyone; tagging, signing, packing a CRX and any upload need credentials only the
owner ([@sneak](https://sneak.berlin)) holds and are marked **owner-only**
below. Releases are tagged from `main` (see the Workflow section of `TODO.md`),
so the "release commit" throughout is the `main` commit the milestone PR merged.
## Procedure
1. **Confirm `main` is green in CI.** The `check` workflow
(`.gitea/workflows/check.yml`) runs `script/cibuild`, i.e. `docker build .`,
and the `Dockerfile` runs `make check` as a build step, so a green `check`
run is a green `make check`. Find the run for the exact release commit on the
tracker's Actions view. _Check:_ that commit's `check` run succeeded; running
`make check` on a clean checkout of the commit reproduces it and exits 0.
2. **Confirm the version matches the intended tag.** `package.json`,
`manifest/chrome.json` and `manifest/firefox.json` must all declare the same
`X.Y.Z`. `make build` fails when they disagree, but nothing checks that they
equal the tag you mean to create — that is this manual step. _Check:_ all
three files read the same `X.Y.Z`, and it is the version you intend to tag
`vX.Y.Z`.
3. **Build and package from a clean checkout of that commit.** From a fresh
clone, or a working tree with no local modifications (`git status` clean),
checked out at the release commit: run `make setup`, then `make package`.
`make package` runs `make build` first, so the archives can only be made from
a `dist/` verified against that build's own receipt as a release (not debug)
build. It writes three files into `release/`:
`autistmask-chrome-<version>.zip`, `autistmask-firefox-<version>.xpi`, and
`SHA256SUMS`. _Check:_ those three files exist and `<version>` in the archive
names is the version confirmed in step 2. The Firefox `.xpi` is **unsigned**
(see step 6 and "Installing on Firefox" in `README.md`).
4. **Verify `SHA256SUMS`.** The archives are deterministic — sorted entries,
fixed timestamps, fixed compression — so a second `make package` from another
clean checkout of the same commit produces byte-identical files. Verify the
recorded digests against the files with `sha256sum -c SHA256SUMS`, run from
`release/`. To confirm reproducibility, run `make package` again on a
separate clean checkout and compare the digests. _Check:_ `sha256sum -c`
reports `OK` for every file, and an independent build's digests match.
5. **Tag the release commit.** _(owner-only)_ Create an annotated tag `vX.Y.Z`
on the release commit and push it: `git tag -a vX.Y.Z` (with a message), then
`git push origin vX.Y.Z`. _Check:_ `git tag` lists `vX.Y.Z`, and
`git rev-parse vX.Y.Z^{commit}` resolves to the release commit.
6. **Distribute per browser.** _(owner-only; pending the owner's choice on
https://git.eeqj.de/sneak/AutistMask/issues/386)_ How 1.0.0 is distributed on
each browser is not yet decided; it is the open question on that issue, and
the concrete steps cannot be written until the owner records a choice there.
These steps need credentials only the owner holds. The options under
consideration are:
- **Firefox** — the packaged `.xpi` is unsigned, and release Firefox and ESR
refuse an unsigned add-on:
- (a) AMO self-distribution signing (unlisted): submit the `.xpi` to AMO
with the owner's credentials; AMO returns a signed `.xpi` installable
on every Firefox, with nothing listed publicly.
- (b) AMO listed: as (a), plus a public AMO listing and review.
- (c) Ship the unsigned `.xpi` and state that Firefox support means
Developer Edition, Nightly, or an Unbranded build with
`xpinstall.signatures.required` set to `false`.
- **Chrome** — the repo packs no CRX and publishes nothing; the extension id
is fixed by the `key` in `manifest/chrome.json`:
- (a) Chrome Web Store (unlisted): upload the `.zip` with the owner's
developer account; the store delivers installs and updates.
- (b) Self-hosted CRX signed with the private key the owner holds
(`chrome --pack-extension=dist/chrome --pack-extension-key=<path to the .pem>`),
installable only via enterprise policy on Windows and macOS, so
realistically Linux-only.
- (c) "Load unpacked" from `dist/chrome/` only, as today.
Once the owner decides, the chosen steps — including which credentials they
need and who holds them — are written into this section and `README.md`'s
installation sections are updated to match, which is part of the definition
of done of https://git.eeqj.de/sneak/AutistMask/issues/386. _Check:_ for a
store or AMO route, the artifact installs from the store or AMO on a clean
browser profile; for the CRX or unpacked route, the documented load succeeds
and Chrome reports the extension id `gipbhkogfopeahplcjhipkgpcimdpkip`.
-29
View File
@@ -37,10 +37,6 @@ DISCARD_DIST="$ROOT/script/discard-dist-on-failure"
MARKER_ON="autistmask-build-debug=on" MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off" MARKER_OFF="autistmask-build-debug=off"
# The same test recovery phrase verify-build searches release bundles for. Held
# here too, the way the markers above are, so a case can plant it in a bundle.
TEST_MNEMONIC="cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
RECEIPT_HEADER="autistmask-build-receipt v1" RECEIPT_HEADER="autistmask-build-receipt v1"
NEWLINE=' NEWLINE='
@@ -520,24 +516,6 @@ c_debug_build() {
write_receipt write_receipt
} }
# A release bundle that still carries the test recovery phrase — the regression
# verify-build guards against, and the reason DEBUG_MNEMONIC is behind the
# __BUILD_DEBUG__ define in src/shared/constants.js. The receipt is regenerated
# so the phrase is caught as bundle content, not incidentally as a stale digest.
c_release_bundle_with_mnemonic() {
printf '/* %s */\n' "$TEST_MNEMONIC" >>dist/chrome/src/popup/index.js
write_receipt
}
# The same phrase in a debug build is expected: make build-debug ships it on
# purpose, so the phrase check must stay quiet under --expect debug.
c_debug_bundle_with_mnemonic() {
write_bundle dist/chrome/src/popup/index.js "$MARKER_ON"
write_bundle dist/firefox/src/popup/index.js "$MARKER_ON"
printf '/* %s */\n' "$TEST_MNEMONIC" >>dist/chrome/src/popup/index.js
write_receipt
}
c_no_dist() { rm -rf dist; } c_no_dist() { rm -rf dist; }
# --- dist discard ----------------------------------------------------------- # --- dist discard -----------------------------------------------------------
@@ -775,13 +753,6 @@ run_cases() {
check_case "debug bundles under --expect debug pass" \ check_case "debug bundles under --expect debug pass" \
no debug 0 "2 bundle(s) $MARKER_ON" c_debug_build no debug 0 "2 bundle(s) $MARKER_ON" c_debug_build
check_case "release bundle carrying the test recovery phrase fails" \
no release 1 \
"carries the BIP-39 test recovery phrase" c_release_bundle_with_mnemonic
check_case "debug bundle carrying the test recovery phrase passes" \
no debug 0 "2 bundle(s) $MARKER_ON" c_debug_bundle_with_mnemonic
check_case "no --expect argument" \ check_case "no --expect argument" \
no no-expect 1 "no --expect argument." c_control no no-expect 1 "no --expect argument." c_control
-27
View File
@@ -13,12 +13,6 @@
# fallback branch; the property only exists in the emitted output, so it has to # fallback branch; the property only exists in the emitted output, so it has to
# be asserted against the emitted output. # be asserted against the emitted output.
# #
# The DEBUG half also checks the phrase directly: a release build must not carry
# the test recovery phrase in any emitted file. The phrase is behind the
# __BUILD_DEBUG__ define in src/shared/constants.js and folds away in a release
# build, but the marker only proves DEBUG compiled off, not that the fold
# removed the string; the phrase grep is the assertion that it did.
#
# Which mode to expect is an ARGUMENT (--expect release|debug) and is never # Which mode to expect is an ARGUMENT (--expect release|debug) and is never
# taken from this script's environment. It used to be read from # taken from this script's environment. It used to be read from
# AUTISTMASK_DEBUG here, which meant an operator with AUTISTMASK_DEBUG=1 # AUTISTMASK_DEBUG here, which meant an operator with AUTISTMASK_DEBUG=1
@@ -73,15 +67,6 @@ TAB=' '
MARKER_ON="autistmask-build-debug=on" MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off" MARKER_OFF="autistmask-build-debug=off"
# The 12-word BIP-39 test recovery phrase from src/shared/constants.js. It is
# behind the __BUILD_DEBUG__ define there, so a release build folds it out of
# every bundle; this is the assertion that it stayed out. The phrase is a
# publicly committed test value rather than a secret, but a BIP-39 phrase in a
# distributed wallet artifact is exactly the string a scanner or auditor has to
# stop and reason about, so a release build must not ship it. A debug build
# ships it on purpose, so this is checked only when release is expected.
TEST_MNEMONIC="cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
RECEIPT_HEADER="autistmask-build-receipt v1" RECEIPT_HEADER="autistmask-build-receipt v1"
# Set by the arguments. # Set by the arguments.
@@ -298,18 +283,6 @@ check_entry() {
receipt records $ENTRY_HASH and the file on disk is $SHA. Something wrote receipt records $ENTRY_HASH and the file on disk is $SHA. Something wrote
to dist/ after the build, so this artifact is not the one that was built." to dist/ after the build, so this artifact is not the one that was built."
# No emitted file of a release build may carry the test recovery phrase.
# Checked on every file, not only the audited bundles, so a copy that
# reached some other emitted file fails here too. A debug build ships the
# phrase deliberately, so this runs only when release was expected.
if [ "$EXPECT" = "$MARKER_OFF" ] && has_marker "$TEST_MNEMONIC" "$ENTRY_PATH"; then
fail "$ENTRY_PATH carries the BIP-39 test recovery phrase, which a
release build must fold out. The __BUILD_DEBUG__ define in build.js is what
drops it from src/shared/constants.js; check that DEBUG_MNEMONIC is still
behind that flag. A recovery phrase in a distributed bundle is exactly the
string an auditor or scanner has to stop on, so this is a hard failure."
fi
if [ "$ENTRY_FLAG" = A ]; then if [ "$ENTRY_FLAG" = A ]; then
read_marker "$ENTRY_PATH" read_marker "$ENTRY_PATH"
[ "$MARKER" = "$EXPECT" ] || [ "$MARKER" = "$EXPECT" ] ||
-9
View File
@@ -1344,15 +1344,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;
// This message signs and broadcasts a transaction, so it is honoured
// only for a transaction approval. A sign or connection approval
// carries no approvedTx, and reaching the broadcast path with one used
// to fail closed by throwing deeper in; refusing here keeps a future
// refactor from turning that incidental throw into a live path, and
// keeps a reject on this message from retiring an approval of another
// kind.
if (approval.type !== "tx") return false;
// A reject arriving while an attempt holds the approval is refused, // A reject arriving while an attempt holds the approval is refused,
// not honoured: the attempt is on its way to broadcasting the // not honoured: the attempt is on its way to broadcasting the
// transaction, and resolving 4001 here would tell the page the request // transaction, and resolving 4001 here would tell the page the request
-15
View File
@@ -152,21 +152,6 @@
<!-- Shared password fields --> <!-- Shared password fields -->
<div class="mb-2" id="add-wallet-password-section"> <div class="mb-2" id="add-wallet-password-section">
<!-- Shown only when the profile already holds a wallet:
each wallet has its own password (its own
encryptedSecret), so a second wallet does not reuse
the first one's. addWallet.js toggles this on screen
entry from state.wallets.length, so it is constant
while the screen is up and moves nothing. -->
<p
class="text-xs mb-2 border border-border border-dashed p-2 hidden"
id="add-wallet-separate-password-note"
>
You already have a wallet. Each wallet has its own
password: the one you choose here is only for this new
wallet, and it need not match any password you already
use.
</p>
<label class="block mb-1">Choose a password</label> <label class="block mb-1">Choose a password</label>
<!-- The hint is swapped in place when the import tab <!-- The hint is swapped in place when the import tab
changes, and it sits directly above the password changes, and it sits directly above the password
-15
View File
@@ -100,24 +100,9 @@ function clear() {
$("add-wallet-phrase-warning").style.visibility = "hidden"; $("add-wallet-phrase-warning").style.visibility = "hidden";
} }
// Each wallet has its own password (its own encryptedSecret), so adding a
// second wallet does not reuse the first one's. The note that says so is
// only meaningful once a wallet exists — on the first wallet there is no
// other password to be separate from — so it is shown only then. This is
// decided on entry and stays put while the screen is up, so it does not
// move the password fields the way a per-tab hint would.
function updateSeparatePasswordNote() {
const hasExistingWallet = state.wallets.length > 0;
$("add-wallet-separate-password-note").classList.toggle(
"hidden",
!hasExistingWallet,
);
}
function show() { function show() {
clear(); clear();
switchMode("mnemonic"); switchMode("mnemonic");
updateSeparatePasswordNote();
showView("add-wallet"); showView("add-wallet");
} }
+24 -51
View File
@@ -30,7 +30,6 @@ const {
displayedDecimals, displayedDecimals,
transferAmountUnits, transferAmountUnits,
} = require("../../shared/transferAmount"); } = require("../../shared/transferAmount");
const { assertWithinCeilings } = require("../../shared/approvalVerify");
const { const {
CODES, CODES,
FEE_PENDING, FEE_PENDING,
@@ -395,46 +394,6 @@ async function estimateGas(txInfo) {
} }
} }
// Populate the transaction this send describes, enforce the fee bound against
// the fees that were actually filled in, then sign and broadcast it. The send
// pins no fee fields, so ethers fills maxFeePerGas and the gas limit from what
// the configured RPC node answers, with nothing otherwise bounding what a
// hostile node can set — the dApp path's ceilings never reached this one.
// Populating before the check is what makes assertWithinCeilings() see the
// same numbers that would be signed; it throws an ApprovalMismatchError when
// the product gasLimit × maxFeePerGas is over the bound, which the caller
// shows in the reserved error area rather than sending.
async function populateVerifyAndSend(connectedSigner, tx) {
let request;
if (tx.token === "ETH") {
request = { to: tx.to, value: parseEther(tx.amount) };
} else {
const contract = new Contract(tx.token, ERC20_ABI, connectedSigner);
// The contract's decimals() is read to be COMPARED with the scale the
// screen rendered this amount at, not to encode with: encoding from it
// signs whatever the contract answers now, which is not what the user
// read. A disagreement throws. See transferAmount.js.
const amount = transferAmountUnits(
tx.amount,
tx.tokenDecimals,
await contract.decimals(),
);
request = await contract.transfer.populateTransaction(tx.to, amount);
}
const populated = await connectedSigner.populateTransaction(request);
assertWithinCeilings(populated);
return connectedSigner.sendTransaction(populated);
}
// Show a full-sentence send failure in the reserved errors box, the same
// element and markup renderValidation() uses for messages carrying the user's
// own numbers, so it never moves anything on the screen.
function showSendError(message) {
const el = $("confirm-errors");
el.innerHTML = `<div class="text-xs">${escapeHtml(message)}</div>`;
el.style.visibility = "visible";
}
async function checkRecipientHistory(txInfo) { async function checkRecipientHistory(txInfo) {
try { try {
const provider = getProvider(state.rpcUrl, state.networkId); const provider = getProvider(state.rpcUrl, state.networkId);
@@ -508,7 +467,29 @@ function init(_ctx) {
const provider = getProvider(state.rpcUrl, state.networkId); const provider = getProvider(state.rpcUrl, state.networkId);
const connectedSigner = signer.connect(provider); const connectedSigner = signer.connect(provider);
tx = await populateVerifyAndSend(connectedSigner, pendingTx); if (pendingTx.token === "ETH") {
tx = await connectedSigner.sendTransaction({
to: pendingTx.to,
value: parseEther(pendingTx.amount),
});
} else {
const contract = new Contract(
pendingTx.token,
ERC20_ABI,
connectedSigner,
);
// The contract's decimals() is read to be COMPARED with the
// scale the screen rendered this amount at, not to encode with:
// encoding from it signs whatever the contract answers now,
// which is not what the user read. A disagreement throws and is
// reported on the error screen. See transferAmount.js.
const amount = transferAmountUnits(
pendingTx.amount,
pendingTx.tokenDecimals,
await contract.decimals(),
);
tx = await contract.transfer(pendingTx.to, amount);
}
// Best-effort: clear decrypted secret after use. // Best-effort: clear decrypted secret after use.
// Note: JS strings are immutable; this nulls the reference but // Note: JS strings are immutable; this nulls the reference but
@@ -517,14 +498,6 @@ function init(_ctx) {
txStatus.showWait(pendingTx, tx.hash); txStatus.showWait(pendingTx, tx.hash);
} catch (e) { } catch (e) {
decryptedSecret = null; decryptedSecret = null;
// A fee over the bound is refused before anything is broadcast, so
// there is no transaction that may have reached the network to warn
// about: the message stays on the confirmation screen where the
// user can go back, rather than routing to the sent/failed screen.
if (e && e.approvalMismatch) {
showSendError(e.message);
return;
}
const hash = tx ? tx.hash : null; const hash = tx ? tx.hash : null;
txStatus.showError(pendingTx, hash, e.shortMessage || e.message); txStatus.showError(pendingTx, hash, e.shortMessage || e.message);
} finally { } finally {
@@ -538,4 +511,4 @@ function init(_ctx) {
}); });
} }
module.exports = { init, show, restore, populateVerifyAndSend }; module.exports = { init, show, restore };
+7 -14
View File
@@ -51,12 +51,16 @@ function clear() {
// The lost-password screen holds no secret — a wallet name is not one — // The lost-password screen holds no secret — a wallet name is not one —
// but it is wiped on leave for the neighbouring reason: a typed // but it is wiped on leave for the neighbouring reason: a typed
// confirmation left standing in a hidden view is one click away from // confirmation left standing in a hidden view is one click away from
// destroying a wallet the user has since navigated off. // destroying a wallet the user has since navigated off. The button is
// re-enabled here too, so a screen left mid-delete is usable on re-entry.
function clearLostPassword() { function clearLostPassword() {
lostPasswordIndex = null; lostPasswordIndex = null;
$("delete-wallet-lost-name-input").value = ""; $("delete-wallet-lost-name-input").value = "";
$("delete-wallet-lost-flash").textContent = ""; $("delete-wallet-lost-flash").textContent = "";
$("delete-wallet-lost-flash").style.visibility = "hidden"; $("delete-wallet-lost-flash").style.visibility = "hidden";
const btn = $("btn-delete-wallet-lost-confirm");
btn.disabled = false;
btn.classList.remove("text-muted");
} }
function show(walletIdx) { function show(walletIdx) {
@@ -94,17 +98,6 @@ function showLostPassword() {
// cleanup and the accountsChanged broadcast cannot drift apart between // cleanup and the accountsChanged broadcast cannot drift apart between
// them. // them.
async function finishDelete(walletIdx) { async function finishDelete(walletIdx) {
// Each route's confirm button was disabled by its own click handler
// before the delete ran. Re-enable both here, on the one path they
// share, so the two routes reset the same way and a second delete in
// the same popup session finds a live button instead of a dead one.
const passwordBtn = $("btn-delete-wallet-confirm");
passwordBtn.disabled = false;
passwordBtn.classList.remove("text-muted");
const lostPasswordBtn = $("btn-delete-wallet-lost-confirm");
lostPasswordBtn.disabled = false;
lostPasswordBtn.classList.remove("text-muted");
const { activeAddressChanged } = removeWalletFromState(state, walletIdx); const { activeAddressChanged } = removeWalletFromState(state, walletIdx);
deleteWalletIndex = null; deleteWalletIndex = null;
@@ -194,8 +187,8 @@ function init(_ctx) {
btn.disabled = true; btn.disabled = true;
btn.classList.add("text-muted"); btn.classList.add("text-muted");
// finishDelete() re-enables the button; navigating away then runs // finishDelete() navigates, and the leave hook re-enables the
// the leave hook that wipes the typed name. // button and wipes the typed name on the way out.
await finishDelete(lostPasswordIndex); await finishDelete(lostPasswordIndex);
}); });
+1 -6
View File
@@ -12,7 +12,6 @@
// escapeHtml lives in src/shared/html.js, where the escape and the // escapeHtml lives in src/shared/html.js, where the escape and the
// reasoning behind it are; it is re-exported below so views keep importing // reasoning behind it are; it is re-exported below so views keep importing
// it from here. // it from here.
const { DEBUG } = require("../../shared/constants");
const { escapeHtml } = require("../../shared/html"); const { escapeHtml } = require("../../shared/html");
const { isDebug } = require("../../shared/log"); const { isDebug } = require("../../shared/log");
const { formatUsd, getPrice } = require("../../shared/prices"); const { formatUsd, getPrice } = require("../../shared/prices");
@@ -120,11 +119,7 @@ function updateDebugBanner(viewName) {
"background:#c00;color:#fff;text-align:center;font-size:10px;padding:1px 0;font-family:monospace;position:sticky;top:0;z-index:9999;"; "background:#c00;color:#fff;text-align:center;font-size:10px;padding:1px 0;font-family:monospace;position:sticky;top:0;z-index:9999;";
document.body.prepend(banner); document.body.prepend(banner);
} }
// The view id is internal vocabulary; it helps while developing but const suffix = viewName ? " (" + viewName + ")" : "";
// means nothing to a user. Only a debug build appends it, gated on the
// compile-time DEBUG constant so a release build never shows it — not
// isDebug(), which is also true for a testnet or the runtime toggle.
const suffix = DEBUG && viewName ? " (" + viewName + ")" : "";
if (debug && net.isTestnet) { if (debug && net.isTestnet) {
banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix; banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix;
} else if (net.isTestnet) { } else if (net.isTestnet) {
+2 -35
View File
@@ -51,7 +51,6 @@
const { const {
Transaction, Transaction,
accessListify, accessListify,
formatEther,
getAddress, getAddress,
getBytes, getBytes,
verifyMessage, verifyMessage,
@@ -135,19 +134,10 @@ const FORBIDDEN_FIELDS = [
const MAX_GAS_LIMIT = 100000000n; const MAX_GAS_LIMIT = 100000000n;
// 100,000 gwei per gas: orders of magnitude above the highest fee either // 100,000 gwei per gas: orders of magnitude above the highest fee either
// supported network has produced. // supported network has produced, and low enough to catch a fee that would
// hand the validator the balance.
const MAX_FEE_PER_GAS = 100000000000000n; const MAX_FEE_PER_GAS = 100000000000000n;
// The largest total fee this wallet will sign, in wei. The two ceilings above
// bound the gas limit and the price per gas each on its own, but the fee a
// validator is actually paid is their product, and a gas limit and a price
// that are each under their own ceiling still multiply to thousands of ETH —
// 30,000,000 gas at 100,000 gwei is about 3,000 ETH. Bounding the product is
// what catches a fee that would hand the validator the balance; the per-field
// ceilings alone do not. A full 30,000,000-gas block at 33 gwei reaches this,
// which no ordinary wallet transaction approaches.
const MAX_TOTAL_FEE = 1000000000000000000n; // 1 ETH
// A refusal to act on an artifact: it is not the thing that was approved, so // A refusal to act on an artifact: it is not the thing that was approved, so
// the approval it was offered against is spent and must not be retried. Every // the approval it was offered against is spent and must not be retried. Every
// throw in this module is one of these; the background distinguishes them from // throw in this module is one of these; the background distinguishes them from
@@ -394,28 +384,6 @@ function assertWithinCeilings(tx) {
); );
} }
} }
// The product: gasLimit × the most this transaction could pay per gas —
// maxFeePerGas for a type-2 transaction, gasPrice for a legacy or type-1
// one. This is the fee a gas-consuming contract can really extract, and it
// is the bound the two per-field ceilings above cannot express.
if (present(tx.gasLimit)) {
const gasLimit = normalizeQuantity(tx.gasLimit, "gas limit");
let price = null;
if (present(tx.maxFeePerGas)) {
price = normalizeQuantity(tx.maxFeePerGas, "maximum fee per gas");
} else if (present(tx.gasPrice)) {
price = normalizeQuantity(tx.gasPrice, "gas price");
}
if (price !== null && gasLimit * price > MAX_TOTAL_FEE) {
throw refuse(
"This transaction would allow a network fee of up to " +
formatEther(gasLimit * price) +
" ETH, which is more than the " +
formatEther(MAX_TOTAL_FEE) +
" ETH this wallet will sign for.",
);
}
}
} }
// Refuse a field only a transaction type this wallet does not sign can carry. // Refuse a field only a transaction type this wallet does not sign can carry.
@@ -807,5 +775,4 @@ module.exports = {
TX_STAGE_NONCE, TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
}; };
+2 -9
View File
@@ -22,15 +22,8 @@ const BUILD_DEBUG_MARKER = DEBUG
? "autistmask-build-debug=on" ? "autistmask-build-debug=on"
: "autistmask-build-debug=off"; : "autistmask-build-debug=off";
// Behind DEBUG for the same reason BUILD_DEBUG_MARKER is above: in a release const DEBUG_MNEMONIC =
// build __BUILD_DEBUG__ is a compile-time false, esbuild drops this branch, and "cube evolve unfold result inch risk jealous skill hotel bulb night wreck";
// the phrase never reaches a distributed bundle. The literal used to survive as
// dead text because module.exports keeps this const live even though wallet.js's
// only use of it is folded away; making the value itself fold to null removes
// it. script/verify-build fails a release build if the phrase appears anyway.
const DEBUG_MNEMONIC = DEBUG
? "cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
: null;
const ETHEREUM_MAINNET_CHAIN_ID = "0x1"; const ETHEREUM_MAINNET_CHAIN_ID = "0x1";
const ETHEREUM_SEPOLIA_CHAIN_ID = "0xaa36a7"; const ETHEREUM_SEPOLIA_CHAIN_ID = "0xaa36a7";
-58
View File
@@ -1,58 +0,0 @@
// Adding a second wallet accepts a password different from the first one's
// with nothing on screen saying the two are separate — each wallet has its
// own encryptedSecret, so per-wallet passwords are by design, but the add
// screen said only "Choose a password"
// (https://git.eeqj.de/sneak/AutistMask/issues/374).
//
// The fix is copy: a note on the password screen that says each wallet has
// its own password and this one need not match. It is only meaningful once
// a wallet exists — on the very first wallet there is no other password to
// be separate from — so it is shown then and hidden otherwise. These boot
// the real popup and reach the add-wallet screen through the same button a
// user presses, so the note's visibility is decided by the real show().
const {
bootPopup,
cleanupPopup,
unversionedValidProfile,
POPUP_HTML,
} = require("./support/popupBoot");
const NOTE = "add-wallet-separate-password-note";
afterEach(() => {
cleanupPopup();
});
describe("second-wallet password note", () => {
test("hidden while onboarding the first wallet", async () => {
const page = await bootPopup(undefined);
expect(page.pageErrors).toEqual([]);
await page.click("btn-welcome-add");
expect(page.visibleViews()).toContain("add-wallet");
expect(page.hidden(NOTE)).toBe(true);
});
test("shown when a wallet already exists", async () => {
const page = await bootPopup(unversionedValidProfile());
expect(page.pageErrors).toEqual([]);
await page.click("btn-main-add-wallet");
expect(page.visibleViews()).toContain("add-wallet");
expect(page.hidden(NOTE)).toBe(false);
});
// The copy states the two facts the definition of done asks for — each
// wallet has its own password, and this one need not match — and stays
// consistent with the no-password-reset reality of
// https://git.eeqj.de/sneak/AutistMask/issues/312 by not promising any
// recovery or reset here.
test("the note says the password is per-wallet and need not match", () => {
const note = /id="add-wallet-separate-password-note"[^>]*>([^]*?)<\/p>/
.exec(POPUP_HTML)[1]
.replace(/\s+/g, " ")
.trim();
expect(note).toContain("its own");
expect(note).toContain("need not match");
expect(note).not.toMatch(/recover|reset/i);
});
});
-18
View File
@@ -212,24 +212,6 @@ describe("prepareApprovalTx", () => {
).rejects.toThrow(/gas limit no network this wallet supports/); ).rejects.toThrow(/gas limit no network this wallet supports/);
}); });
// The combined bound at population: a gas limit and a fee that are each
// under their own ceiling but multiply to thousands of ETH is refused
// before the approval window opens, so the user is never shown a
// balance-draining fee to click past.
test("refuses a fee whose product with the gas limit is over the bound", async () => {
const gouging = providerWith({
estimateGas: async () => 30000000n,
getFeeData: async () => ({
gasPrice: MAX_FEE_PER_GAS,
maxFeePerGas: MAX_FEE_PER_GAS,
maxPriorityFeePerGas: 1000000000n,
}),
});
await expect(
prepareApprovalTx(gouging, signer.address, TX_PARAMS),
).rejects.toThrow(/network fee of up to/);
});
// No approval and no window: the failure goes back to the page the click // No approval and no window: the failure goes back to the page the click
// came from, in a sentence. // came from, in a sentence.
test("reports a failed estimate as a full sentence", async () => { test("reports a failed estimate as a full sentence", async () => {
-114
View File
@@ -28,7 +28,6 @@ const {
TX_STAGE_NONCE, TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify"); } = require("../src/shared/approvalVerify");
const { prepareApprovalTx } = require("../src/shared/approvalTx"); const { prepareApprovalTx } = require("../src/shared/approvalTx");
const { getSignerForAddress } = require("../src/shared/wallet"); const { getSignerForAddress } = require("../src/shared/wallet");
@@ -476,131 +475,18 @@ describe("verifySignedTx field comparison", () => {
assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }), assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }),
).toThrow(/fee per gas far above any plausible value/); ).toThrow(/fee per gas far above any plausible value/);
} }
// Each field at its own ceiling multiplies to about 10,000 ETH, which
// is exactly the combination the per-field ceilings cannot see and the
// product bound is for: it is refused, not accepted.
expect(() => expect(() =>
assertWithinCeilings({ assertWithinCeilings({
gasLimit: MAX_GAS_LIMIT, gasLimit: MAX_GAS_LIMIT,
maxFeePerGas: MAX_FEE_PER_GAS, maxFeePerGas: MAX_FEE_PER_GAS,
maxPriorityFeePerGas: MAX_FEE_PER_GAS, maxPriorityFeePerGas: MAX_FEE_PER_GAS,
}), }),
).toThrow(/network fee of up to/);
// An ordinary transaction — a modest gas limit and a modest fee, each
// far under its ceiling and their product far under the bound — passes.
expect(() =>
assertWithinCeilings({
gasLimit: 21000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
}),
).not.toThrow(); ).not.toThrow();
// Nothing to bound is not a failure: a type 2 approval carries no gas // Nothing to bound is not a failure: a type 2 approval carries no gas
// price, and a bare object must not be refused for lacking one. // price, and a bare object must not be refused for lacking one.
expect(() => assertWithinCeilings({})).not.toThrow(); expect(() => assertWithinCeilings({})).not.toThrow();
}); });
// The defect this issue closes: gasLimit and maxFeePerGas each under their
// own ceiling, but their product — the fee a gas-consuming contract can
// really extract — thousands of ETH. The per-field ceilings accept it; the
// product bound refuses it, on either side of the screen.
describe("the combined fee bound", () => {
// A gas limit and a fee that are each comfortably under their own
// ceiling but multiply to well over 1 ETH: 30,000,000 gas at 100,000
// gwei is about 3,000 ETH.
const OVER = { gasLimit: 30000000n, maxFeePerGas: 100000000000000n };
test("each field is under its own ceiling", () => {
expect(OVER.gasLimit).toBeLessThan(MAX_GAS_LIMIT);
expect(OVER.maxFeePerGas).toBeLessThanOrEqual(MAX_FEE_PER_GAS);
expect(OVER.gasLimit * OVER.maxFeePerGas).toBeGreaterThan(
MAX_TOTAL_FEE,
);
});
test("assertWithinCeilings refuses the product over the bound", () => {
expect(() =>
assertWithinCeilings({
...OVER,
maxPriorityFeePerGas: 1000000000n,
}),
).toThrow(/network fee of up to 3000\.0 ETH/);
});
test("assertWithinCeilings bounds a legacy gasPrice the same way", () => {
expect(() =>
assertWithinCeilings({
gasLimit: OVER.gasLimit,
gasPrice: OVER.maxFeePerGas,
}),
).toThrow(/network fee of up to/);
});
// The boundary itself, pinned rather than only some value well past
// it. Both fields stay under their own ceilings, so it is the product
// and nothing else that decides these two cases: a gas limit of 10,000
// at the per-gas ceiling is exactly 1 ETH.
test("assertWithinCeilings accepts a product exactly at the bound and refuses one wei over", () => {
expect(MAX_FEE_PER_GAS * 10000n).toBe(MAX_TOTAL_FEE);
expect(() =>
assertWithinCeilings({
gasLimit: 10000n,
maxFeePerGas: MAX_FEE_PER_GAS,
}),
).not.toThrow();
expect(() =>
assertWithinCeilings({
gasLimit: 10001n,
maxFeePerGas: MAX_FEE_PER_GAS,
}),
).toThrow(/network fee of up to/);
});
// The dApp path: an artifact whose fee is within each field's ceiling
// but over the product bound, both displayed and signed, is refused at
// verification just as it is at population.
test("verifySignedTx refuses an over-bound product even when displayed", async () => {
const raw = await signedWith(OVER);
expect(() =>
verifySignedTx(
raw,
approvedFor(TX_PARAMS, OVER),
signer.address,
SELECTED,
),
).toThrow(/network fee of up to/);
});
test("verifySignedTx accepts a product just under the bound", async () => {
// 21,000 gas at 40 gwei is 0.00084 ETH — an ordinary send.
const under = { gasLimit: 21000n, maxFeePerGas: 40000000000n };
expect(under.gasLimit * under.maxFeePerGas).toBeLessThan(
MAX_TOTAL_FEE,
);
const raw = await signedWith(under);
expect(() =>
verifySignedTx(
raw,
approvedFor(TX_PARAMS, under),
signer.address,
SELECTED,
),
).not.toThrow();
});
test("the refusal names the fee and the limit in a full sentence", () => {
try {
assertWithinCeilings(OVER);
throw new Error("expected a rejection");
} catch (e) {
expect(e.approvalMismatch).toBe(true);
expect(e.message).toMatch(/^[A-Z].*\.$/);
expect(e.message).toContain("3000.0 ETH");
expect(e.message).toContain("1.0 ETH");
}
});
});
test("every field mismatch is a refusal, not a warning", async () => { test("every field mismatch is a refusal, not a warning", async () => {
const raw = await signedWith({ nonce: 8 }); const raw = await signedWith({ nonce: 8 });
try { try {
-143
View File
@@ -1541,149 +1541,6 @@ describe("a claimed approval outlives every other retirement path", () => {
}); });
}); });
// The approval popup connects a port named for its approval whatever the
// approval's kind, so a decision or a disconnect on that port can reach a
// transaction approval. Both must be declined: the port decides only
// site-connection approvals, and settling a transaction approval it does not
// own — while an attempt is broadcasting behind it — is the round-3 fund-loss
// bug, where the page is told the request was rejected as the transaction goes
// out. These three paths route through settleApproval() and, before this
// suite, were exercised only against site approvals.
describe("the site-connection port never retires a transaction approval", () => {
async function txMidBroadcast() {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
return { bg, pending, id, inFlight, first };
}
test("an approve on the port does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).decide(true, false);
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
test("a reject on the port does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).decide(false, false);
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
test("a port disconnect does not settle it", async () => {
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
bg.connectApproval(id).disconnect();
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
});
});
// AUTISTMASK_TX_RESPONSE signs and broadcasts a transaction, so it is honoured
// only for a transaction approval. A reject shaped as this message used to
// retire a sign or connection approval outright, and an approve carrying a
// signed artifact used to run the broadcast path against an approval that names
// no transaction, failing closed only by throwing deeper in.
describe("a transaction response is honoured only for a transaction approval", () => {
test("a reject does not retire a sign approval", async () => {
const bg = loadBackground();
const pending = bg.requestSign();
await settle();
const id = pending.id();
bg.send(
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
// Still live: its own reject settles it.
bg.send(
{ type: "AUTISTMASK_SIGN_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
test("a reject does not retire a connection approval", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const id = pending.id();
bg.send(
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
// Still live: the port that owns it connects the site.
const port = bg.connectApproval(id);
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
});
test("an approve carrying a signed transaction never broadcasts against a sign approval", async () => {
const bg = loadBackground();
const pending = bg.requestSign();
await settle();
const id = pending.id();
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
expect(pending.result()).toBeNull();
});
});
// A handler that throws must still answer. `sendResponse` is the only thing // A handler that throws must still answer. `sendResponse` is the only thing
// that settles the page's window.ethereum.request() promise, so a throw that // that settles the page's window.ethereum.request() promise, so a throw that
// escapes a handler leaves that promise pending forever — no error, no // escapes a handler leaves that promise pending forever — no error, no
-100
View File
@@ -1,100 +0,0 @@
// The wallet's OWN send path enforces the same combined fee bound the dApp
// path does (https://git.eeqj.de/sneak/AutistMask/issues/399).
//
// The send in src/popup/views/confirmTx.js pins no fee fields, so ethers fills
// maxFeePerGas and the gas limit from whatever the configured RPC node
// answers. Nothing bounded that: a hostile node could report a fee whose
// product with the gas limit is thousands of ETH, and it would be both
// displayed and signed. populateVerifyAndSend() populates the transaction and
// runs assertWithinCeilings() on the populated fees before signing, so an
// over-bound send is refused before anything is broadcast.
//
// The check is driven here with a fake connected signer rather than a real
// one: populateTransaction() returns the fees the node would have produced,
// and sendTransaction() records whether the send actually happened. The real
// DOM path around it — reading the fee error into the reserved errors box — is
// covered by the Chrome e2e suite.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
const { populateVerifyAndSend } = require("../src/popup/views/confirmTx");
const {
MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify");
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
// A signer whose populateTransaction() fills in the fees a node quoted and
// whose sendTransaction() records the call, so a test can assert whether the
// send was reached at all.
function fakeSigner(fees) {
const sent = [];
return {
sent,
populateTransaction: async (request) => ({
...request,
from: RECIPIENT,
nonce: 0,
type: 2,
chainId: 1n,
gasLimit: fees.gasLimit,
maxFeePerGas: fees.maxFeePerGas,
maxPriorityFeePerGas: 1000000000n,
}),
sendTransaction: async (tx) => {
sent.push(tx);
return { hash: "0xabc" };
},
};
}
const ETH_SEND = { token: "ETH", to: RECIPIENT, amount: "1.0" };
describe("populateVerifyAndSend enforces the combined fee bound", () => {
// A gas limit and a fee that are each under their own field ceiling, but
// multiply to about 3,000 ETH — the combination the per-field ceilings
// cannot see.
const OVER = { gasLimit: 30000000n, maxFeePerGas: MAX_FEE_PER_GAS };
test("each field is under its ceiling but the product is over the bound", () => {
expect(OVER.maxFeePerGas).toBeLessThanOrEqual(MAX_FEE_PER_GAS);
expect(OVER.gasLimit * OVER.maxFeePerGas).toBeGreaterThan(
MAX_TOTAL_FEE,
);
});
test("refuses an over-bound send without broadcasting it", async () => {
const signer = fakeSigner(OVER);
let thrown;
try {
await populateVerifyAndSend(signer, ETH_SEND);
} catch (e) {
thrown = e;
}
expect(thrown).toBeDefined();
expect(thrown.approvalMismatch).toBe(true);
expect(thrown.message).toMatch(/^[A-Z].*\.$/);
expect(thrown.message).toContain("3000.0 ETH");
expect(thrown.message).toContain("1.0 ETH");
// The one guarantee that matters: nothing was signed or sent.
expect(signer.sent).toHaveLength(0);
});
test("broadcasts a send whose product is just under the bound", async () => {
// 21,000 gas at 40 gwei is 0.00084 ETH — an ordinary send.
const under = { gasLimit: 21000n, maxFeePerGas: 40000000000n };
expect(under.gasLimit * under.maxFeePerGas).toBeLessThan(MAX_TOTAL_FEE);
const signer = fakeSigner(under);
const tx = await populateVerifyAndSend(signer, ETH_SEND);
expect(tx.hash).toBe("0xabc");
expect(signer.sent).toHaveLength(1);
expect(signer.sent[0].gasLimit).toBe(under.gasLimit);
});
});
-59
View File
@@ -1,59 +0,0 @@
// Tests for the debug/testnet banner (issue #375).
//
// On a testnet the banner is raised even in a release build, but it must not
// append the active view's internal id: the user should see "[TESTNET]", never
// "[TESTNET] (approve-tx)". The suffix is gated on the compile-time DEBUG
// constant, which is false in a plain test load, so this drives exactly the
// text a shipped build renders. Revert the gate to the old unconditional
// suffix and this fails.
//
// The banner is created on demand by updateDebugBanner(); the document stub
// records what it prepends so the assertion can read the resulting text.
function makeBanner() {
return {
id: "",
textContent: "",
style: { cssText: "" },
remove() {},
};
}
function makeDocument() {
let banner = null;
return {
getElementById(id) {
return id === "debug-banner" ? banner : null;
},
createElement: () => makeBanner(),
body: {
prepend(node) {
banner = node;
},
},
};
}
function load() {
jest.resetModules();
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
globalThis.document = makeDocument();
const helpers = require("../src/popup/views/helpers");
const { state } = require("../src/shared/state");
return { helpers, state };
}
describe("the release banner on a testnet", () => {
test("carries no internal view id", () => {
const { helpers, state } = load();
state.networkId = "sepolia";
helpers.updateDebugBanner("approve-tx");
expect(
globalThis.document.getElementById("debug-banner").textContent,
).toBe("[TESTNET]");
});
});
+5 -65
View File
@@ -172,15 +172,6 @@ async function openLostPassword(deleteWallet, walletIdx) {
await click("btn-delete-wallet-lost-password"); await click("btn-delete-wallet-lost-password");
} }
// Delete a wallet through the password route: open its confirm screen,
// enter the password, and confirm. The vault is mocked, so the password
// text itself is irrelevant — decryptWithPassword decides pass or fail.
async function deleteWithPassword(deleteWallet, walletIdx) {
deleteWallet.show(walletIdx);
node("delete-wallet-password").value = "any password";
await click("btn-delete-wallet-confirm");
}
// ------------------------------------------------------------ tests // ------------------------------------------------------------ tests
// The stub is what every persistence assertion below rests on, so its one // The stub is what every persistence assertion below rests on, so its one
@@ -465,21 +456,15 @@ describe("what the screen leaves behind", () => {
); );
}); });
// Both routes now re-enable through finishDelete(), not their leave // Left mid-delete, the screen has to come back usable.
// hooks, so the button comes back live once a delete completes. test("the confirm button is re-enabled on the way out", async () => {
test("the confirm button is re-enabled after a delete", async () => { const { helpers, deleteWallet } = load();
const { deleteWallet } = load();
await openLostPassword(deleteWallet, 1); await openLostPassword(deleteWallet, 1);
node("delete-wallet-lost-name-input").value = "Wallet 2"; node("btn-delete-wallet-lost-confirm").disabled = true;
await click("btn-delete-wallet-lost-confirm"); helpers.showView("settings");
expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false); expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false);
expect(
node("btn-delete-wallet-lost-confirm").classList.contains(
"text-muted",
),
).toBe(false);
}); });
// A wallet name is not a secret, so the screen is excluded for the // A wallet name is not a secret, so the screen is excluded for the
@@ -490,48 +475,3 @@ describe("what the screen leaves behind", () => {
expect(RESTORABLE_VIEWS.has("delete-wallet-confirm")).toBe(false); expect(RESTORABLE_VIEWS.has("delete-wallet-confirm")).toBe(false);
}); });
}); });
// The password route is the pre-existing bug this file's fix addresses:
// its Confirm Delete button was disabled before the decrypt and never
// re-enabled on success, so a second delete in the same popup session
// found a dead button. Now both routes re-enable through finishDelete().
//
// Against head these tests fail: with the re-enable absent, the button
// stays disabled after the first delete, so the disabled assertions read
// true where they expect false.
describe("the password route's confirm button", () => {
test("is re-enabled after a successful delete", async () => {
const { deleteWallet, vault } = load();
vault.decryptWithPassword.mockResolvedValue();
await deleteWithPassword(deleteWallet, 1);
expect(node("btn-delete-wallet-confirm").disabled).toBe(false);
expect(
node("btn-delete-wallet-confirm").classList.contains("text-muted"),
).toBe(false);
});
// The reported symptom: delete one wallet, then open Delete Wallet for
// a second one without reopening the popup. The button must be live on
// that second visit, and the second delete must actually persist.
test("a second delete works in the same popup session", async () => {
const { deleteWallet, vault, storage } = load();
vault.decryptWithPassword.mockResolvedValue();
await deleteWithPassword(deleteWallet, 1);
// Wallet 2 is gone; the list is now [Wallet 1, Wallet 3]. Opening
// the confirm screen for the wallet now at index 1 (Wallet 3) must
// find its button live, not the dead one the first delete left.
deleteWallet.show(1);
expect(node("btn-delete-wallet-confirm").disabled).toBe(false);
node("delete-wallet-password").value = "any password";
await click("btn-delete-wallet-confirm");
expect((await persistedWallets(storage)).map((w) => w.name)).toEqual([
"Wallet 1",
]);
});
});
+8 -17
View File
@@ -89,28 +89,19 @@ describe("chrome extension identity", () => {
// The private half is a credential. It has never been in this repo and no // The private half is a credential. It has never been in this repo and no
// target generates one into the working tree; this fails loudly if that // target generates one into the working tree; this fails loudly if that
// ever changes, because a committed private key is one anyone can sign a // ever changes, because a committed .pem is a key anyone can sign a CRX
// CRX with under this extension's id. // with under this extension's id.
//
// Matched by CONTENT, not by filename: a key committed as notes.txt or with
// no extension carries the same risk as one named key.pem, and a
// filename-only check waves it through. The PEM header a private key opens
// with is the signature searched for. The pattern does not trip on its own
// source: the bracket-expression characters between the two anchors are not
// in the character class, so this file is not a match for it.
const PRIVATE_KEY_HEADER = /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----/;
test("no private key is committed anywhere in the tree", () => { test("no private key is committed anywhere in the tree", () => {
const root = path.join(__dirname, "..");
const tracked = require("child_process") const tracked = require("child_process")
.execSync("git ls-files", { cwd: root, encoding: "utf8" }) .execSync("git ls-files", {
cwd: path.join(__dirname, ".."),
encoding: "utf8",
})
.split("\n") .split("\n")
.filter(Boolean); .filter(Boolean);
const offenders = tracked.filter((f) => expect(tracked.filter((f) => /\.(pem|key|p12|pfx)$/i.test(f))).toEqual(
PRIVATE_KEY_HEADER.test( [],
fs.readFileSync(path.join(root, f), "latin1"),
),
); );
expect(offenders).toEqual([]);
}); });
}); });
-8
View File
@@ -27,14 +27,6 @@ describe("generateMnemonic in a release build", () => {
expect(constants.DEBUG).toBe(false); expect(constants.DEBUG).toBe(false);
}); });
test("the test phrase folds away when DEBUG is false", () => {
// The release bundle is what must not carry the phrase; here, with the
// define absent, DEBUG_MNEMONIC is the null branch the bundler keeps,
// and the literal only exists in the branch it drops.
const { constants } = loadWallet();
expect(constants.DEBUG_MNEMONIC).toBeNull();
});
test("returns fresh, valid 12-word phrases that are not the test phrase", () => { test("returns fresh, valid 12-word phrases that are not the test phrase", () => {
const { constants, wallet } = loadWallet(); const { constants, wallet } = loadWallet();