From 09b602579a48bff78a42b43fe9ecd201ee22e645 Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 12:03:39 +0200 Subject: [PATCH] fix: one password-failure message across every screen (closes #172) A rejected password was reported three different ways depending on which screen you were on, including the fragment "Wrong password." which is not a sentence. All six decryptWithPassword call sites now show the same full sentence. Strings only -- a wrong password still fails closed on every screen and still resolves no pending approval. A test pins the invariant per call site: each decryptWithPassword call is walked out to its enclosing try and forward to that block's catch, and the prose shown there must equal the canonical sentence. Per-file matching was not enough, since a file with two call sites kept passing while one of them diverged. --- TODO.md | 8 ++ src/popup/views/confirmTx.js | 5 +- src/popup/views/deleteWallet.js | 3 +- src/popup/views/exportPrivkey.js | 2 +- src/popup/views/showPhrase.js | 2 +- tests/exportPrivkey.test.js | 2 +- tests/passwordMessages.test.js | 213 +++++++++++++++++++++++++++++++ 7 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 tests/passwordMessages.test.js diff --git a/TODO.md b/TODO.md index 1e673b5..e398fd3 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,14 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-12: One wording for a rejected password on every screen that asks for + one — the send confirmation and the delete-wallet confirmation no longer say + "Wrong password." (a fragment, which `RULES.md` Language & Labeling forbids) + and the two reveal screens no longer say "not correct", so all five + `decryptWithPassword` call sites now show the sentence the dApp approval paths + introduced. Strings only, no behaviour change, and each error container + measured at a 360px viewport in the pinned Playwright container + ([#172](https://git.eeqj.de/sneak/AutistMask/issues/172)). - 2026-08-12: Closed the empty-array hole in the end-to-end unstubbed-request guard. `batch.every()` is vacuously true on `[]`, so a POST with body `[]` was answered `200 []` instead of failing the suite; the guard now rejects an empty diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 03678f4..5d59734 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -422,7 +422,10 @@ function init(ctx) { password, ); } catch (e) { - showError("confirm-tx-password-error", "Wrong password."); + showError( + "confirm-tx-password-error", + "That password is incorrect. Please try again.", + ); return; } diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js index 78e440c..685f937 100644 --- a/src/popup/views/deleteWallet.js +++ b/src/popup/views/deleteWallet.js @@ -74,7 +74,8 @@ function init(_ctx) { try { await decryptWithPassword(wallet.encryptedSecret, pw); } catch (_e) { - $("delete-wallet-flash").textContent = "Wrong password."; + $("delete-wallet-flash").textContent = + "That password is incorrect. Please try again."; $("delete-wallet-flash").style.visibility = "visible"; btn.disabled = false; btn.classList.remove("text-muted"); diff --git a/src/popup/views/exportPrivkey.js b/src/popup/views/exportPrivkey.js index 6860240..dd60f51 100644 --- a/src/popup/views/exportPrivkey.js +++ b/src/popup/views/exportPrivkey.js @@ -144,7 +144,7 @@ async function reveal() { $("export-privkey-flash").style.visibility = "hidden"; } catch { if (!isCurrentReveal(generation)) return; - fail("That password is not correct. Please try again."); + fail("That password is incorrect. Please try again."); } finally { btn.disabled = false; btn.classList.remove("text-muted"); diff --git a/src/popup/views/showPhrase.js b/src/popup/views/showPhrase.js index 80ad327..d8962c2 100644 --- a/src/popup/views/showPhrase.js +++ b/src/popup/views/showPhrase.js @@ -126,7 +126,7 @@ async function reveal() { 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."); + fail("That password is incorrect. Please try again."); } finally { btn.disabled = false; btn.classList.remove("text-muted"); diff --git a/tests/exportPrivkey.test.js b/tests/exportPrivkey.test.js index 468acca..516026b 100644 --- a/tests/exportPrivkey.test.js +++ b/tests/exportPrivkey.test.js @@ -247,7 +247,7 @@ describe("a reveal that is not interrupted", () => { expect(node("export-privkey-value").textContent).toBe(""); expect(node("export-privkey-flash").textContent).toBe( - "That password is not correct. Please try again.", + "That password is incorrect. Please try again.", ); }); }); diff --git a/tests/passwordMessages.test.js b/tests/passwordMessages.test.js new file mode 100644 index 0000000..494891c --- /dev/null +++ b/tests/passwordMessages.test.js @@ -0,0 +1,213 @@ +// One wording for one condition (issue #172). +// +// Every screen that asks for the password decrypts the vault itself, and +// each one used to write its own sentence for the same failure: the send +// confirmation and the delete-wallet confirmation said "Wrong password." +// (a fragment, which RULES.md Language & Labeling forbids), the reveal +// screens said "That password is not correct.", and the two dApp approval +// paths said "That password is incorrect." A user hitting two of those +// minutes apart had no way to tell whether the wallet meant the same +// thing. +// +// This scans the source rather than driving six views, because the +// invariant is about the set of call sites and not about any one of them: +// a seventh screen that decrypts the vault has to join the set, and a +// DOM test per view cannot notice one that was never written. +// +// The assertions are per CALL SITE, not per file. approval.js decrypts in +// two places and is where the divergence came from; a per-file check that +// only asks whether the canonical sentence appears somewhere in the file +// passes while one of those two says something else entirely. So each +// call site is read back to its own catch handler and the prose that +// handler shows the user must be the canonical sentence and nothing else +// — which fails on a novel wording, not only on a known-superseded one. + +const fs = require("fs"); +const path = require("path"); + +const SRC = path.join(__dirname, "..", "src"); + +const CANONICAL = "That password is incorrect. Please try again."; + +// Wordings this repo has actually shipped for the same condition. This is +// a secondary, whole-file sweep for stragglers outside a decrypt handler; +// divergence at a call site is caught by the exact-match assertion, which +// needs no list of phrasings to guess at. +const SUPERSEDED = [ + "Wrong password.", + "That password is not correct. Please try again.", +]; + +function jsFilesUnder(dir) { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return jsFilesUnder(full); + return entry.name.endsWith(".js") ? [full] : []; + }); +} + +// Blank out the interior of every comment and string literal, keeping the +// offsets and line breaks, so braces can be counted without a quote or a +// commented-out block throwing the count off. The literals are returned +// alongside with the offset of their opening quote, which is how a +// message is later attributed to the handler it sits in. +function scan(source) { + const masked = source.split(""); + const strings = []; + const blank = (from, to) => { + for (let k = from; k < to; k++) if (masked[k] !== "\n") masked[k] = " "; + }; + let i = 0; + while (i < source.length) { + const two = source.slice(i, i + 2); + if (two === "//") { + const nl = source.indexOf("\n", i); + const stop = nl === -1 ? source.length : nl; + blank(i, stop); + i = stop; + } else if (two === "/*") { + const close = source.indexOf("*/", i + 2); + const stop = close === -1 ? source.length : close + 2; + blank(i, stop); + i = stop; + } else if ( + source[i] === '"' || + source[i] === "'" || + source[i] === "`" + ) { + const quote = source[i]; + let j = i + 1; + let value = ""; + while (j < source.length && source[j] !== quote) { + if (source[j] === "\\") { + value += source[j + 1]; + j += 2; + continue; + } + value += source[j]; + j += 1; + } + blank(i + 1, j); + strings.push({ offset: i, value }); + i = j + 1; + } else { + i += 1; + } + } + return { masked: masked.join(""), strings }; +} + +// Offset of the `{` that opens the block containing `at`, or -1. +function enclosingBlockStart(masked, at) { + let depth = 0; + for (let i = at; i >= 0; i--) { + if (masked[i] === "}") depth += 1; + else if (masked[i] === "{") { + if (depth === 0) return i; + depth -= 1; + } + } + return -1; +} + +// Offset just past the `}` matching the `{` at `open`. +function blockEnd(masked, open) { + let depth = 0; + for (let i = open; i < masked.length; i++) { + if (masked[i] === "{") depth += 1; + else if (masked[i] === "}") { + depth -= 1; + if (depth === 0) return i + 1; + } + } + throw new Error("unterminated block"); +} + +// The catch handler guarding a given decryptWithPassword call: walk out to +// the try block the call sits in, then take the catch that follows it. +function handlerSpan(masked, callOffset, label) { + const tryOpen = enclosingBlockStart(masked, callOffset); + if (tryOpen === -1 || !/\btry\s*$/.test(masked.slice(0, tryOpen))) + throw new Error(`${label}: the decrypt is not inside a try block`); + const rest = masked.slice(blockEnd(masked, tryOpen)); + const catchMatch = /^\s*catch\s*(\([^)]*\)\s*)?\{/.exec(rest); + if (!catchMatch) + throw new Error(`${label}: the decrypt's try block has no catch`); + const catchOpen = blockEnd(masked, tryOpen) + catchMatch[0].length - 1; + return [catchOpen, blockEnd(masked, catchOpen)]; +} + +// The prose the handler puts in front of the user. Element ids, class +// names and visibility keywords are single words; a sentence has a space +// in it, and that is the whole distinction needed here. +function handlerMessages(file, callOffset, label) { + const { masked, strings } = scan(fs.readFileSync(file, "utf8")); + const [from, to] = handlerSpan(masked, callOffset, label); + return strings + .filter((s) => s.offset >= from && s.offset < to) + .map((s) => s.value) + .filter((v) => v.includes(" ")); +} + +// The call sites are found, not listed: the file layout moves (the private +// key export was in addressDetail.js when #172 was filed and is its own +// view now), and a hardcoded list would quietly stop covering a screen it +// no longer names. +function callSites() { + const sites = []; + for (const file of jsFilesUnder(SRC)) { + if (file === path.join(SRC, "shared", "vault.js")) continue; + const { masked } = scan(fs.readFileSync(file, "utf8")); + const rel = path.relative(SRC, file).split(path.sep).join("/"); + let n = 0; + let at = masked.indexOf("decryptWithPassword("); + while (at !== -1) { + n += 1; + sites.push({ file, rel, offset: at, label: `${rel} #${n}` }); + at = masked.indexOf("decryptWithPassword(", at + 1); + } + } + return sites.sort((a, b) => a.label.localeCompare(b.label)); +} + +describe("password failure messages", () => { + const sites = callSites(); + const files = [...new Set(sites.map((s) => s.file))].sort(); + + test("the call sites are found where they are expected", () => { + const counts = {}; + for (const site of sites) + counts[site.rel] = (counts[site.rel] ?? 0) + 1; + expect(counts).toEqual({ + "popup/views/approval.js": 2, + "popup/views/confirmTx.js": 1, + "popup/views/deleteWallet.js": 1, + "popup/views/exportPrivkey.js": 1, + "popup/views/showPhrase.js": 1, + }); + }); + + test("the canonical message is a full sentence", () => { + expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/); + }); + + // Exact equality, per call site: a message that is merely different + // rather than known-obsolete fails here too, which a scan for historic + // wordings cannot do. + test.each(sites.map((s) => [s.label, s]))( + "%s answers a rejected password with the canonical sentence", + (label, site) => { + expect(handlerMessages(site.file, site.offset, label)).toEqual([ + CANONICAL, + ]); + }, + ); + + test.each(files.map((f) => [path.relative(SRC, f), f]))( + "%s carries no superseded wording", + (_rel, file) => { + const source = fs.readFileSync(file, "utf8"); + for (const old of SUPERSEDED) expect(source).not.toContain(old); + }, + ); +});