Compare commits

...

2 Commits

Author SHA1 Message Date
d2d53b8028 fix: one wording for a rejected password on every screen (closes #172)
All checks were successful
check / check (push) Successful in 29s
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 six decryptWithPassword call sites, across the five views that own
one, 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 seventh call site
has to join it, and a per-view test cannot notice a screen nobody wrote
one for. It asserts per call site, not per file — each decrypt is read
back to its own catch handler and the prose that handler shows must be
the canonical sentence and nothing else. approval.js decrypts twice and
is where the divergence came from, so a per-file check that only asks
whether the sentence appears somewhere in the file passes while one of
those two says something novel. Exact equality catches a new wording,
not only a known-superseded one.
2026-08-12 10:00:51 +00:00
18b47cd579 test: close the empty-batch hole in the e2e unstubbed-request guard (closes #187)
Some checks failed
check / check (push) Has been cancelled
The guard that reports unrecognised POST bodies used batch.every(), which is
vacuously true on an empty array, so a POST with body [] was answered 200 []
and escaped the one mechanism whose job is to make unrecognised outbound
traffic fail the suite rather than pass silently. Unreachable in practice
today, which is exactly the qualifier that stops being true later.

The comment explaining the guard also described a mechanism that does not
exist: playwright-core decodes a binary body lossily rather than returning
null, so such a body reaches the JSON parse as mojibake and is reported by the
catch, while only an absent or empty body decodes to null and is reported by
the type guard. Both are reported; the comment now describes the two real
routes.
2026-08-12 11:50:38 +02:00
8 changed files with 255 additions and 13 deletions

16
TODO.md
View File

@@ -44,6 +44,22 @@ undefined identifiers, which is how
# Completed Steps # 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
batch, demonstrated green-before/red-after with a throwaway probe. The comment
claiming `postData()` returns `null` for undecodable bodies was corrected to
the two real paths — an absent or empty body decodes to `null`, a binary body
decodes lossily into invalid JSON
([#187](https://git.eeqj.de/sneak/AutistMask/issues/187)).
- 2026-08-12: The transaction confirmation screen has browser coverage. The - 2026-08-12: The transaction confirmation screen has browser coverage. The
end-to-end suite reaches ConfirmTx for both the native ETH and the ERC-20 path end-to-end suite reaches ConfirmTx for both the native ETH and the ERC-20 path
off a funded-balance fixture, and asserts the pending, funded, over-balance off a funded-balance fixture, and asserts the pending, funded, over-balance

View File

@@ -422,7 +422,10 @@ function init(ctx) {
password, password,
); );
} catch (e) { } catch (e) {
showError("confirm-tx-password-error", "Wrong password."); showError(
"confirm-tx-password-error",
"That password is incorrect. Please try again.",
);
return; return;
} }

View File

@@ -74,7 +74,8 @@ function init(_ctx) {
try { try {
await decryptWithPassword(wallet.encryptedSecret, pw); await decryptWithPassword(wallet.encryptedSecret, pw);
} catch (_e) { } 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"; $("delete-wallet-flash").style.visibility = "visible";
btn.disabled = false; btn.disabled = false;
btn.classList.remove("text-muted"); btn.classList.remove("text-muted");

View File

@@ -144,7 +144,7 @@ async function reveal() {
$("export-privkey-flash").style.visibility = "hidden"; $("export-privkey-flash").style.visibility = "hidden";
} catch { } catch {
if (!isCurrentReveal(generation)) return; if (!isCurrentReveal(generation)) return;
fail("That password is not correct. Please try again."); fail("That password is incorrect. Please try again.");
} finally { } finally {
btn.disabled = false; btn.disabled = false;
btn.classList.remove("text-muted"); btn.classList.remove("text-muted");

View File

@@ -126,7 +126,7 @@ async function reveal() {
if (!isCurrentReveal(generation)) return; if (!isCurrentReveal(generation)) return;
// Deliberately not the caught error: the message is fixed so that // Deliberately not the caught error: the message is fixed so that
// nothing derived from the ciphertext or the attempt can surface. // 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 { } finally {
btn.disabled = false; btn.disabled = false;
btn.classList.remove("text-muted"); btn.classList.remove("text-muted");

View File

@@ -297,17 +297,26 @@ async function handleRpc(route, postData, opts, report) {
// ethers batches by default, so the body may be an array. // ethers batches by default, so the body may be an array.
const batch = Array.isArray(payload) ? payload : [payload]; const batch = Array.isArray(payload) ? payload : [payload];
// Anything that is not a JSON-RPC object, or a batch of them, is not // Anything that is not a JSON-RPC object, or a NON-EMPTY batch of
// RPC at all and must be reported like any other unrecognised // them, is not RPC at all and must be reported like any other
// outbound traffic rather than dereferenced. request.postData() // unrecognised outbound traffic rather than dereferenced.
// returns null both for a bodyless POST and for a body Playwright //
// cannot decode as UTF-8 (sendBeacon with a Blob, or any binary // The length check is not decoration: every() is vacuously true on an
// payload), so this is not an empty-string special case: it rejects // empty array, so without it a POST with body [] was answered 200 []
// every non-object payload, exactly as the catch above rejects every // and escaped the guard entirely (issue #187). No real batch is empty,
// unparseable one. // so nothing legitimate is caught by it.
//
// Two distinct paths land a non-RPC body here, and neither is an
// empty-string special case. playwright-core's postData() is
// `buffer.toString("utf-8") || null`, so an absent or empty body
// decodes to null, JSON.parse("null") yields null, and the type guard
// below reports it. A binary body is instead decoded LOSSILY into
// mojibake — not null — which is not valid JSON, so the catch above
// reports that one. Both end up reported; only the route differs.
if ( if (
payload === null || payload === null ||
typeof payload !== "object" || typeof payload !== "object" ||
batch.length === 0 ||
!batch.every((req) => req !== null && typeof req === "object") !batch.every((req) => req !== null && typeof req === "object")
) { ) {
report("unstubbed request: POST " + route.request().url()); report("unstubbed request: POST " + route.request().url());

View File

@@ -247,7 +247,7 @@ describe("a reveal that is not interrupted", () => {
expect(node("export-privkey-value").textContent).toBe(""); expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-flash").textContent).toBe( expect(node("export-privkey-flash").textContent).toBe(
"That password is not correct. Please try again.", "That password is incorrect. Please try again.",
); );
}); });
}); });

View File

@@ -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);
},
);
});