test(dialogs): guard against aliases of confirmAction and other native forms
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 2m2s

Review of PR #465 (comment 24953), suggestion 1: an alias
(`const c = confirmAction; if (!c(...))`) called confirmAction out of reach
of the "awaited" rule, and window["confirm"](...), confirm?.(...) and
`const { confirm } = window` escaped the native-call rule. Any mention of
confirmAction other than the awaited call, its declaration, a plain named
import or `confirm: confirmAction` is now refused; the three native forms
are flagged. Samples added for each.

Refs #421

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
le king fu 2026-10-18 14:26:19 -04:00
parent 155722775a
commit f75fcee12b

View file

@ -8,10 +8,15 @@
* *
* It reads the non-test sources of `src/`, comments removed, as `i18n-mock-guard.test.ts` does, * It reads the non-test sources of `src/`, comments removed, as `i18n-mock-guard.test.ts` does,
* and fails on: * and fails on:
* 1. a call to `confirm` or `alert`, bare or through `window`, `globalThis` or `self` * 1. a native `confirm` or `alert`: called bare or through `window`, `globalThis` or `self`
* (a member call such as `deps.confirm(` is not the native one); * (a member call such as `deps.confirm(` is not the native one), with `?.(` too, read by
* index (`window["confirm"]`) or destructured from one of those objects;
* 2. a call to `confirmAction(` or to an injected `confirm(` (`deps.confirm(`, * 2. a call to `confirmAction(` or to an injected `confirm(` (`deps.confirm(`,
* `ui.confirm(`) that is not directly preceded by `await`. * `ui.confirm(`) that is not directly preceded by `await`;
* 3. any other mention of `confirmAction`: an alias (`const c = confirmAction`) or a renamed
* import would call it out of reach of rule 2 (#465 review). Allowed besides the awaited
* call: its declaration, a plain named import, and `confirm: confirmAction`, the value of
* an injected `confirm` whose calls rule 2 checks.
* `confirmAction` itself calls the plugin's `ask`, so `src/services/confirmService.ts` is * `confirmAction` itself calls the plugin's `ask`, so `src/services/confirmService.ts` is
* scanned like any other file. Keywords rather than a parse: the samples below check that each * scanned like any other file. Keywords rather than a parse: the samples below check that each
* rule flags what it targets and lets the right form through. * rule flags what it targets and lets the right form through.
@ -23,12 +28,22 @@ import { relative, resolve, sep } from "path";
const SRC = resolve(import.meta.dirname, ".."); const SRC = resolve(import.meta.dirname, "..");
const NATIVE_CALL = /(?:\b(?:window|globalThis|self)\s*\.\s*|(?<![.\w$]))(?:confirm|alert)\s*\(/; const NATIVE_CALL = /(?:\b(?:window|globalThis|self)\s*\.\s*|(?<![.\w$]))(?:confirm|alert)\s*(?:\?\.\s*)?\(/;
const NATIVE_INDEX = /\b(?:window|globalThis|self)\s*(?:\?\.\s*)?\[\s*["'`](?:confirm|alert)["'`]\s*\]/;
const NATIVE_DESTRUCTURED = /\{[^}]*\b(?:confirm|alert)\b[^}]*\}\s*=\s*(?:window|globalThis|self)\b/;
// `confirmAction(` or `<receiver>.confirm(`, the receiver captured to skip the native ones. // `confirmAction(` or `<receiver>.confirm(`, the receiver captured to skip the native ones.
const CONFIRM_CALL = /\b(?:confirmAction|(\w+)\s*\.\s*confirm)\s*\(/g; const CONFIRM_CALL = /\b(?:confirmAction|(\w+)\s*\.\s*confirm)\s*\(/g;
const NATIVE_RECEIVER = /^(?:window|globalThis|self)$/; const NATIVE_RECEIVER = /^(?:window|globalThis|self)$/;
const AWAITED = /\bawait\s+$/; const AWAITED = /\bawait\s+$/;
const DECLARED = /\bfunction\s+$/; const DECLARED = /\bfunction\s+$/;
// Mentions of `confirmAction` that rule 3 allows; any other one is an offence.
const CONFIRM_ACTION = /\bconfirmAction\b/g;
const ALLOWED_MENTION = [
/\bawait\s+confirmAction\s*\(/g,
/\bfunction\s+confirmAction\s*\(/g,
/^\s*import\s*\{\s*confirmAction\s*\}\s*from\s/g,
/\bconfirm\s*:\s*confirmAction\s*[,}]/g,
];
/** The source without its comments, line breaks kept so that line numbers stay right. */ /** The source without its comments, line breaks kept so that line numbers stay right. */
function withoutComments(source: string): string { function withoutComments(source: string): string {
@ -40,12 +55,16 @@ function withoutComments(source: string): string {
/** The offences of one line: a native call, or a confirmation not awaited. */ /** The offences of one line: a native call, or a confirmation not awaited. */
function lineOffences(line: string): string[] { function lineOffences(line: string): string[] {
const found: string[] = []; const found: string[] = [];
if (NATIVE_CALL.test(line)) found.push("native confirm/alert"); if (NATIVE_CALL.test(line) || NATIVE_INDEX.test(line) || NATIVE_DESTRUCTURED.test(line)) {
found.push("native confirm/alert");
}
for (const match of line.matchAll(CONFIRM_CALL)) { for (const match of line.matchAll(CONFIRM_CALL)) {
if (match[1] !== undefined && NATIVE_RECEIVER.test(match[1])) continue; if (match[1] !== undefined && NATIVE_RECEIVER.test(match[1])) continue;
const before = line.slice(0, match.index); const before = line.slice(0, match.index);
if (!AWAITED.test(before) && !DECLARED.test(before)) found.push("confirmation not awaited"); if (!AWAITED.test(before) && !DECLARED.test(before)) found.push("confirmation not awaited");
} }
const allowed = ALLOWED_MENTION.reduce((n, rule) => n + [...line.matchAll(rule)].length, 0);
if ([...line.matchAll(CONFIRM_ACTION)].length > allowed) found.push("confirmAction out of reach");
return found; return found;
} }
@ -78,6 +97,11 @@ describe("native dialog guard (#421)", () => {
"globalThis.confirm(message);", "globalThis.confirm(message);",
"self . alert(message);", "self . alert(message);",
"alert(message);", "alert(message);",
"if (!window[\"confirm\"](message)) return;",
"globalThis['alert'](message);",
"if (!confirm?.(message)) return;",
"const { confirm } = window;",
"const { alert: show, confirm: ask } = globalThis;",
])("flags the native call in %s", (line) => { ])("flags the native call in %s", (line) => {
expect(lineOffences(line)).toContain("native confirm/alert"); expect(lineOffences(line)).toContain("native confirm/alert");
}); });
@ -88,7 +112,17 @@ describe("native dialog guard (#421)", () => {
"if (!deps.confirm(deps.t(key))) return;", "if (!deps.confirm(deps.t(key))) return;",
"const ok = ui.confirm(message);", "const ok = ui.confirm(message);",
])("flags the confirmation not awaited in %s", (line) => { ])("flags the confirmation not awaited in %s", (line) => {
expect(lineOffences(line)).toEqual(["confirmation not awaited"]); expect(lineOffences(line)).toContain("confirmation not awaited");
});
it.each([
"const c = confirmAction;",
"import { confirmAction as ask } from \"../services/confirmService\";",
"const handlers = { remove: confirmAction };",
"items.map(confirmAction);",
"if (await confirmAction(a) && check(confirmAction)) {",
])("flags confirmAction out of reach in %s", (line) => {
expect(lineOffences(line)).toContain("confirmAction out of reach");
}); });
it.each([ it.each([
@ -97,9 +131,11 @@ describe("native dialog guard (#421)", () => {
"if (!(await deps.confirm(deps.t(key)))) return;", "if (!(await deps.confirm(deps.t(key)))) return;",
"const ok = await confirmAction(message);", "const ok = await confirmAction(message);",
"onClick={() => void removeKey({ confirm: confirmAction, t })}", "onClick={() => void removeKey({ confirm: confirmAction, t })}",
"import { confirmAction } from \"../../services/confirmService\";",
"export async function confirmAction(message: string): Promise<boolean> {",
"if ((await confirmAction(a)) && (await confirmAction(b))) {",
"deps.confirm.mockClear();", "deps.confirm.mockClear();",
"const confirmed = await ask(message, { kind: \"warning\" });", "const confirmed = await ask(message, { kind: \"warning\" });",
"export async function confirmAction(message: string): Promise<boolean> {",
"t(\"common.confirm\")", "t(\"common.confirm\")",
])("lets %s through", (line) => { ])("lets %s through", (line) => {
expect(lineOffences(line)).toEqual([]); expect(lineOffences(line)).toEqual([]);