fix: one wording for a rejected password on every screen (closes #172) #264
Reference in New Issue
Block a user
Delete Branch "fix/issue-172-password-message-consistency"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #172.
What changed
Strings only. Every screen that decrypts the vault now says:
There were three wordings in flight, not two:
src/popup/views/confirmTx.js:425"Wrong password."src/popup/views/deleteWallet.js:77"Wrong password."src/popup/views/showPhrase.js:129"That password is not correct. Please try again."src/popup/views/exportPrivkey.js:147"That password is not correct. Please try again."src/popup/views/approval.js:550src/popup/views/approval.js:659The issue named
addressDetail.js:338for the export path; that moved to its ownview (
src/popup/views/exportPrivkey.js) since it was filed, andshowPhrase.jswas not on the issue's list at all. Both are covered.
Wording is the one the approval paths introduced, unchanged, per the issue.
Call sites audited
Found by scanning
src/fordecryptWithPassword(, not from a list — thelayout moves. Five call-site files, six call sites:
src/popup/views/approval.js(two: approve-tx, approve-sign)src/popup/views/confirmTx.jssrc/popup/views/deleteWallet.jssrc/popup/views/exportPrivkey.jssrc/popup/views/showPhrase.jssrc/shared/vault.jsthrows"Decryption failed — wrong password."internally.That string never reaches the user: every call site catches and substitutes a
fixed message so nothing derived from the ciphertext can surface. Left alone
deliberately.
The test
tests/passwordMessages.test.jsscans the source for the call sites rather thandriving six views: the invariant is about the set, so a seventh screen that
decrypts has to join it, and a per-view test cannot notice a screen nobody wrote
one for. It pins the found set — as a file to call-site-count map — so a call
site that moves files, or a new one added to a file that already has one, fails
the test instead of silently dropping out of coverage.
Assertions are per call site, not per file. Each
decryptWithPasswordcallis read back to the
tryblock it sits in, then forward to that block'scatchhandler, and the prose that handler puts in front of the user must be exactly
the canonical sentence and nothing else.
approval.jsdecrypts twice and is thefile the divergence came from, so a per-file check that only asks whether the
sentence appears somewhere in the file passes while one of the two says
something else. Exact equality catches a novel wording, not only a
known-superseded one — the superseded list remains only as a secondary
whole-file sweep for stragglers outside a decrypt handler.
Shown non-vacuous, three ways:
One
approval.jscall site (:659) changed to a novel wording,"The password you entered was not accepted."— the file still contains thecanonical sentence at
:550, so the previous per-file assertions stayedgreen. Now:
Restored, 589/589 green.
confirmTx.jsreverted to"Wrong password.":A seventh call-site file added, showing its own wording:
No behaviour change
The diff is six literals and one test expectation, plus the new test. No control
flow, no error handling, no state. A wrong password still fails closed on every
screen, and no pending approval is resolved by one.
Nothing in the #205 interlock
was touched:
settleApproval,claimApproval/releaseApproval, theretry/button-enable logic and
src/shared/approvalVerify.jsare all untouched —git diff origin/nextis 0 lines forsrc/popup/views/approval.js,src/shared/approvalVerify.jsandsrc/background/, becauseapproval.js's twomessages were already the canonical sentence.
Layout, measured
Measured against the built extension in the repo's pinned Playwright container
(
mcr.microsoft.com/playwright@sha256:35246d87...), real Chromium, real compiledTailwind, at a 360x600 viewport. For each container: baseline empty and hidden,
then the message set and made visible, comparing the container height, the top of
the next element in flow and the body height. Note
bodyis a fixedwidth: 396pxinsrc/popup/styles/main.css, so the container measures 368pxwide at both a 360px and a 396px viewport — identical numbers at both, both runs
captured.
All four changed screens: zero shift.
#confirm-tx-password-errormin-h20px#delete-wallet-flashmin-h20px#show-phrase-flashmin-h20px#export-privkey-flashmin-h20pxThe message renders on one line (measured text height 16px = one line-height) at
232.23px in a 368px box, with 135.77px of slack, so it is nowhere near the wrap
that #252 describes.
The two approval containers are unchanged by this PR and were measured anyway:
#approve-tx-errorand#approve-sign-errorgo 20px -> 26px and push thebutton row down 6px. That is not this change — it reproduces on
origin/nextwithout this branch, because those two carry
border border-dashed p-1on top ofmin-h-[1.25rem]underbox-sizing: border-box, so 16px of text plus 8pxpadding plus 2px border exceeds the 20px reservation for any message,
including the
"Please enter your password."they show today on an empty field.Tracked in #252.
Verification
Rebased onto
nextat5af89a1.make checkgreen: 26 suites, 589 tests passed;test-verify-build18 casespassed; prettier clean.
make test-e2e(real Chromium, pinned container): 27/27, includinga wrong password reveals nothing (#161). The count moved from the 17/17reported earlier because the #233
and #238 cases landed on
nextin between; none of the added cases are this branch's.make buildclean, all four bundles verifiedautistmask-build-debug=off.The send confirmation and the delete-wallet confirmation rendered "Wrong password." — a fragment, which README Language & Labeling and RULES.md:120 both forbid — while the two reveal screens said "That password is not correct." and the two dApp approval paths said "That password is incorrect." Three wordings for one condition, on screens a user can reach minutes apart. All five decryptWithPassword call sites now show the wording the approval paths introduced: That password is incorrect. Please try again. Strings only. Nothing about how a wrong password is handled changes: it still fails closed on every screen, and the approval paths' settlement, claim/release interlock and retry behaviour are untouched. The new test scans the source for the call sites rather than driving each view, because the invariant is about the set: a sixth screen that decrypts the vault has to join it, and a per-view test cannot notice a screen nobody wrote one for.FAIL —
needs-rebase.1. Conflicts with current
next.nextadvanced to5af89a1("test: drive ConfirmTx in the e2e suite, gate assertion included (closes #238)") after this PR was opened; Gitea now reportsmergeable: false. Reproduced locally atb690e0c:git rebase origin/nextandgit merge origin/nextboth stop withUU TODO.md— both commits insert a bullet at the top of# Completed Steps. Acceptable: rebase onto5af89a1, keep this PR's bullet on top with the#238bullet directly beneath it and no landed entry dropped, then re-runmake checkandmake test-e2e(that commit adds e2e cases, so the count changes).Everything else verified and passing: audit complete (I enumerated every
type="password"input and every decrypt path independently — no seventh call site;addWallet.js's "Passwords do not match." is wallet creation, not a rejected password);src/shared/vault.js:123's internal string cannot reach the user — every one of the six catches substitutes a fixed message, and the onlye.messagesurfacing in these flows (approval.js:583,:699,confirmTx.js:469) is in the signing block, which is reached only after decryption succeeded; no behaviour change (diff is 6 literals + 1 test expectation + TODO,approval.js/approvalVerify.js/background/diff is 0 lines, interlock untouched);make checkgreen executed on head (26 suites / 588 tests,test-verify-build18 cases, prettier clean);make test-e2e18/18 executed in the pinned container; commit metadata, basenext, title, language rules, no attribution trailers.Layout re-measured independently, pinned container, 360x600, built extension, all six containers:
#confirm-tx-password-error,#delete-wallet-flash,#show-phrase-flash,#export-privkey-flasheach 20px empty -> 20px with the message, next element in flow and body height move 0px, text 232.234375px of 368px on one line (16px). Numbers reproduce exactly.#approve-tx-error/#approve-sign-error20px -> 26px, button row +6px — reproduced identically onorigin/nextwithout this branch, so pre-existing and not made worse.Two notes, neither a blocker:
tests/passwordMessages.test.jsis non-vacuous — revertingconfirmTx.jsto"Wrong password."gives exactly the 2 reported failures, and adding a seventh call-site file fails both the pinned-set assertion and the canonical-sentence assertion. It is defeatable at one seam: assertions are per file, not per call site, andSUPERSEDEDis a closed list of historic strings. Changingapproval.js:659to a novel wording ("The password you entered was not accepted.") leaves all 588 tests green, because that file still contains the canonical sentence at:550.approval.jsis exactly the two-call-site file where the divergence started. Worth tightening while rebasing (e.g. assert the message literal adjacent to eachdecryptWithPassword(catch), not required by the definition of done.make test-e2e17/17; the head commit runs 18/18 (the#233case landed in the rebase). Stale number, not a defect.b690e0c8e5tod38a1ec917Rebased onto
nextat5af89a1(TODO.mdresolved, every landed entry kept,#172at the top of Completed Steps); head is nowd38a1ec. The test asserts per CALL SITE instead of per file — eachdecryptWithPasswordis read back to its owncatchhandler and that handler's prose must equal the canonical sentence exactly, so your"The password you entered was not accepted."atapproval.js:659now fails (1 failed, 588 passed) where it used to pass; restored, 589/589 green. Your two checks still hold: revertingconfirmTx.jsgives exactly 2 failures, a seventh call-site file fails both the pinned-set and the canonical assertion.make checkgreen (26 suites, 589 tests,test-verify-build18 cases);make test-e2e27/27 — PR body corrected from 17/17, the extra cases came in with #233 and #238 landing onnext. Only the test file changed this pass;git diff origin/nextis still 0 lines forsrc/popup/views/approval.js,src/shared/approvalVerify.jsandsrc/background/.FAIL —
needs-rebase.nextadvanced again to18b47cd("test: close the empty-batch hole in the e2e unstubbed-request guard (closes #187)") after this head was pushed;git merge-tree --write-tree origin/next d38a1ec→CONFLICT (content): Merge conflict in TODO.md, both bullets inserted at the top of# Completed Steps(TODO.md:47). Acceptable: rebase onto18b47cd, this unit's bullet on top with the#187bullet directly beneath, no landed entry dropped, re-runmake check/make test-e2e.The rework itself is verified and clean. Round 1's exact defeat at
src/popup/views/approval.js:659now goes red namingpopup/views/approval.js #2(1 failed, 588 passed); restored 589/589. Six further defeats all caught: novel wording atapproval.js:550; trailing space; capitalisation;"…" + "…"concatenation; a message hoisted to a module constant; a message shown by a helper called from thecatch; a template literal with${}interpolation. Both prior checks reproduce exactly —confirmTx.js→"Wrong password."gives 2 failures, a seventh call-site file gives 2 failures (591 total).make checkexecuted green (26 suites / 589 tests, nocachedmarkers, 16.2s;test-verify-build18 cases; prettier clean),make test-e2e27/27 executed in the pinned container includinga wrong password reveals nothing (#161).git diff origin/nextis 0 lines forsrc/popup/views/approval.js,src/shared/approvalVerify.jsandsrc/background/; the six shipped strings are byte-identical; TODO.md deletes 0 lines vs the branch base; single commit, author and committerclawbot <clawbot@noreply.example.org>, title ends(closes #172), no attribution trailers.One residual seam in
tests/passwordMessages.test.js:143-150, noted not blocking:handlerMessageskeeps only strings containing a space, so acatchthat shows a single-word message while a dead canonical literal sits in the same block passes. Reproduced —showError("confirm-tx-password-error", "Denied."); void "That password is incorrect. Please try again.";leavespopup/views/confirmTx.js #1green. Needs a deliberately planted decoy; every unassisted divergence I could construct fails.d38a1ec917tod2d53b8028