diff --git a/README.md b/README.md index 7a049bd..df15591 100644 --- a/README.md +++ b/README.md @@ -107,9 +107,12 @@ unavailable). The suite lives in `tests/e2e/` and is driven by `playwright-core`, whose version must stay matched to the container's Playwright version — the browsers ship inside the image. -It covers popup load, wallet creation through the UI, the Add Token screen and -the transaction detail screen for an ERC-20 transfer. All outbound network is -intercepted at the browser level and served from fixtures in +It covers popup load, wallet creation through the UI, the Add Token screen, the +transaction detail screen for an ERC-20 transfer, and the recovery phrase screen +— 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, and that reopening the popup does not land on it. All outbound +network is intercepted at the browser level and served from fixtures in `tests/e2e/network.js`, so the run is deterministic and fully offline; unrecognised outbound requests are reported as failures rather than silently allowed. @@ -375,8 +378,11 @@ runtime debug mode is on, or when the active network is a testnet. They are not repeated in the element lists below. Closing and reopening the popup returns to the screen the user was last on only -for the views listed in `RESTORABLE_VIEWS` (`src/popup/index.js`). Every other -screen, including ExportPrivKey, falls back to Home. +for the views listed in `RESTORABLE_VIEWS` (`src/popup/restorableViews.js`). +Every other screen falls back to Home. The screens that display a secret — +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`) @@ -676,8 +682,9 @@ screen, including ExportPrivKey, falls back to Home. - **When**: User tapped the Settings gear. - **Elements**: - "Back" button, "Settings" heading - - Wallets: one row per wallet with its name (tap to rename inline) and an - `[x]` delete button, plus a "+ Add wallet" button + - Wallets: one row per wallet with its name (tap to rename inline), a + `[recovery phrase]` button on HD wallets only, and an `[x]` delete button, + plus a "+ Add wallet" button - Tracked Tokens: one row per tracked token with an `[x]` remove button, plus a "+ Add token" button - Display: "Show tracked tokens with zero balance" checkbox and a Theme @@ -702,6 +709,7 @@ screen, including ExportPrivKey, falls back to Home. - **Transitions**: - "+ Add wallet" → **AddWallet** - "+ Add token" → **SettingsAddToken** + - `[recovery phrase]` on an HD wallet → **ShowRecoveryPhrase** - `[x]` on a wallet → **DeleteWallet** - Tap wallet name → inline rename field (no screen change) - `[x]` on a tracked token or a site → removes it in place (no screen @@ -709,6 +717,32 @@ screen, including ExportPrivKey, falls back to Home. - Ten clicks on the version → reveals the Debug well (no screen change) - "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. The screen is not restorable, so reopening the popup lands on Home + rather than back on the phrase. + #### DeleteWallet (`delete-wallet-confirm`) - **When**: User tapped the `[x]` next to a wallet in Settings. @@ -1148,7 +1182,7 @@ Currently supported: - [x] Delete wallet (with confirmation) - [ ] Delete address from HD wallet (with confirmation) -- [ ] Show wallet's recovery phrase (requires password) +- [x] Show wallet's recovery phrase (requires password) ### Transactions diff --git a/TODO.md b/TODO.md index 608523b..1187492 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,10 @@ undefined identifiers, which is how # Completed Steps +- 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: README Screen Map rebuilt from the code — every screen, element and transition re-verified against `src/popup/` ([#164](https://git.eeqj.de/sneak/AutistMask/issues/164)). diff --git a/src/popup/index.html b/src/popup/index.html index a38e901..361e864 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1098,6 +1098,52 @@ + + + `; }); container.innerHTML = html; @@ -111,6 +120,14 @@ function renderWalletListSettings() { }); }); + container.querySelectorAll(".btn-show-phrase").forEach((btn) => { + btn.addEventListener("click", () => { + const idx = parseInt(btn.dataset.idx, 10); + pushCurrentView(); + showPhrase.show(idx); + }); + }); + // Inline rename on click container.querySelectorAll(".settings-wallet-name").forEach((span) => { span.addEventListener("click", () => { @@ -191,6 +208,7 @@ function renderSiteLists() { function init(ctx) { deleteWallet.init(ctx); + showPhrase.init(); $("btn-save-rpc").addEventListener("click", async () => { const url = $("settings-rpc").value.trim(); diff --git a/src/popup/views/showPhrase.js b/src/popup/views/showPhrase.js new file mode 100644 index 0000000..feb539b --- /dev/null +++ b/src/popup/views/showPhrase.js @@ -0,0 +1,122 @@ +// 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. +// 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, +} = require("./helpers"); +const { state } = require("../../shared/state"); +const { decryptWithPassword } = require("../../shared/vault"); +const { walletHasRecoveryPhrase } = require("../../shared/wallet"); + +const VIEW = "show-phrase"; + +let walletIndex = null; + +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; + $("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); + 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"); + try { + const phrase = await decryptWithPassword( + wallet.encryptedSecret, + password, + ); + $("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 { + // 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 }; diff --git a/src/shared/wallet.js b/src/shared/wallet.js index 8b2dadc..66d760a 100644 --- a/src/shared/wallet.js +++ b/src/shared/wallet.js @@ -74,6 +74,15 @@ function 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 = { generateMnemonic, deriveAddressFromXpub, @@ -83,4 +92,5 @@ module.exports = { addressFromPrivateKey, getSignerForAddress, isValidMnemonic, + walletHasRecoveryPhrase, }; diff --git a/tests/e2e/harness.js b/tests/e2e/harness.js index f51fe78..b080d7d 100644 --- a/tests/e2e/harness.js +++ b/tests/e2e/harness.js @@ -255,6 +255,11 @@ async function openPopup(ctx, popupUrl) { // Full wallet creation through the real UI: BIP-39 generation, libsodium // 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) { await page.click("#btn-welcome-add"); await visible(page, "#view-add-wallet"); @@ -263,10 +268,12 @@ async function createWallet(page) { const el = document.getElementById("wallet-mnemonic"); 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-confirm", PASSWORD); await page.click("#btn-add-wallet-confirm"); await visible(page, "#view-main", 60000); + return phrase; } // Reach the address detail screen from wherever the popup restored to. @@ -281,6 +288,7 @@ async function openAddressDetail(page) { } module.exports = { + PASSWORD, createWallet, launch, openAddressDetail, diff --git a/tests/e2e/run.js b/tests/e2e/run.js index a771e3c..e10058a 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -10,6 +10,7 @@ "use strict"; const { + PASSWORD, createWallet, launch, openAddressDetail, @@ -56,7 +57,11 @@ test("popup loads and reaches the welcome view", async (env) => { }); test("wallet creation through the UI reaches the main view", async (env) => { - await createWallet(env.page); + env.phrase = await createWallet(env.page); + assert( + env.phrase.split(/\s+/).length >= 12, + "wallet creation did not yield a recovery phrase", + ); const addrCount = await env.page .locator("#wallet-list .btn-addr-info") .count(); @@ -117,6 +122,204 @@ test("transaction detail renders an ERC-20 transfer (#151)", async (env) => { 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"); +}); + +// 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 async function main() { @@ -154,6 +357,9 @@ async function main() { popupUrl: session.popupUrl, routeOpts, 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 diff --git a/tests/showPhrase.test.js b/tests/showPhrase.test.js new file mode 100644 index 0000000..d7cf2dd --- /dev/null +++ b/tests/showPhrase.test.js @@ -0,0 +1,94 @@ +// 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/); + }); +});