Some checks failed
check / check (push) Has been cancelled
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.
214 lines
8.1 KiB
JavaScript
214 lines
8.1 KiB
JavaScript
// 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);
|
|
},
|
|
);
|
|
});
|