diff --git a/e2e/backup-restore-journey.spec.ts b/e2e/backup-restore-journey.spec.ts index 9104ec5f6..581fdb204 100644 --- a/e2e/backup-restore-journey.spec.ts +++ b/e2e/backup-restore-journey.spec.ts @@ -319,7 +319,7 @@ test.describe("Backup and restore, through the settings surfaces", () => { tamperedCopyId = null; }); - test("a record is written, sealed into an HLX1 archive, and snapshotted", async ({ + test("a record is written, sealed into an HLX1 archive, and snapshotted @critical", async ({ page, }) => { await page.goto("/measurements", { waitUntil: "domcontentloaded" }); @@ -505,7 +505,7 @@ test.describe("Backup and restore, through the settings surfaces", () => { expect(await storedMeasurements()).toEqual(before); }); - test("the snapshot puts back what was deleted and drops what came after", async ({ + test("the snapshot puts back what was deleted and drops what came after @critical", async ({ page, }) => { const record = requireSeeded(); diff --git a/e2e/delegated-writes.spec.ts b/e2e/delegated-writes.spec.ts index 5c7cfa565..f63b54f5c 100644 --- a/e2e/delegated-writes.spec.ts +++ b/e2e/delegated-writes.spec.ts @@ -136,7 +136,7 @@ test.describe("delegated writes", () => { ).toBeVisible(); } - test("the owner invites at a level that may add entries", async () => { + test("the owner invites at a level that may add entries @critical", async () => { await openInviteForm(); const identifier = ownerPage.locator( @@ -183,7 +183,9 @@ test.describe("delegated writes", () => { ).toBe(WRITE_LEVEL_VALUE); }); - test("the delegate accepts and opens the record", async ({ page }) => { + test("the delegate accepts and opens the record @critical", async ({ + page, + }) => { await page.goto("/settings/access"); await page.locator('[data-slot="grant-accept"]').first().click(); @@ -203,7 +205,9 @@ test.describe("delegated writes", () => { await expect(banner).not.toContainText("read it, not change it"); }); - test("the reading they add lands in the owner's record", async ({ page }) => { + test("the reading they add lands in the owner's record @critical", async ({ + page, + }) => { await page.goto("/measurements"); await expect( page.locator('[data-slot="shared-record-banner"]'), diff --git a/e2e/medication-compliance-journey.spec.ts b/e2e/medication-compliance-journey.spec.ts index 83f01b3dc..809f7120e 100644 --- a/e2e/medication-compliance-journey.spec.ts +++ b/e2e/medication-compliance-journey.spec.ts @@ -363,7 +363,7 @@ test.describe("medication adherence journey", () => { }); }); - test("a taken and a skipped dose move the card, the history and the dashboard tile", async ({ + test("a taken and a skipped dose move the card, the history and the dashboard tile @critical", async ({ page, }) => { // The wizard is a seven-step dialog with per-step validation gating Next; diff --git a/e2e/pwa-offline.spec.ts b/e2e/pwa-offline.spec.ts index 05701ce0d..28b34c16a 100644 --- a/e2e/pwa-offline.spec.ts +++ b/e2e/pwa-offline.spec.ts @@ -25,7 +25,7 @@ import { MOBILE_ROUTES_STORAGE_STATE_PATH } from "./setup/global-setup"; test.describe("offline, with the service worker", () => { test.use({ storageState: MOBILE_ROUTES_STORAGE_STATE_PATH }); - test("an open page says it is offline, a new one opens the offline page, and both recover", async ({ + test("an open page says it is offline, a new one opens the offline page, and both recover @critical", async ({ page, context, }) => { diff --git a/e2e/setup/critical-fixture.ts b/e2e/setup/critical-fixture.ts new file mode 100644 index 000000000..07c07b9b0 --- /dev/null +++ b/e2e/setup/critical-fixture.ts @@ -0,0 +1,166 @@ +/** + * Accounts for the `@critical` journeys: one per journey AND per project. + * + * The `@critical` journeys run against the real server with no `/api` stubs + * (`src/__tests__/e2e-critical-no-api-stub-guard.test.ts` holds that line), so + * every verdict they read is a statement about a real record. Two things would + * make those verdicts somebody else's: + * + * - a sibling spec writing into the same account. The shared fixture account + * is read and written by sixty files, so "the dashboard tile shows the + * weight I just entered" is only true of an account nobody else writes; + * - the same journey running in the other project. Every other journey that + * owns its account answers that by running in ONE project (the long list in + * `playwright.config.ts`). These journeys are the release gate and are + * meant to hold at both widths, so instead of dropping a project each + * project gets its own account: `chromium-desktop` and `chromium-mobile` + * run the same file in parallel and never meet. + * + * The accounts are seeded and signed in by `global-setup.ts`, like every + * other fixture account; this module only names them and resets what the + * journeys write. It talks to Postgres through `pg` for the reason + * `global-setup.ts` gives (Playwright's loader cannot load the generated + * Prisma client). + */ +import { resolve } from "node:path"; + +import type { TestInfo } from "@playwright/test"; +import pg from "pg"; + +/** The journeys that own an account pair. */ +export const CRITICAL_JOURNEYS = [ + "auth", + "measurement", + "mood", + "labs", + "wellbeing", + "settings", +] as const; + +export type CriticalJourney = (typeof CRITICAL_JOURNEYS)[number]; + +/** The projects the journeys run in; each gets its own account per journey. */ +export const CRITICAL_PROJECTS = { + "chromium-desktop": "d", + "chromium-mobile": "m", +} as const; + +type CriticalProject = keyof typeof CRITICAL_PROJECTS; + +export interface CriticalAccount { + username: string; + email: string; + password: string; + storageStatePath: string; +} + +function accountFor( + journey: CriticalJourney, + suffix: (typeof CRITICAL_PROJECTS)[CriticalProject], +): CriticalAccount { + const username = `e2e-critical-${journey}-${suffix}`; + return { + username, + email: `${username}@healthlog.test`, + // Fixture credential for a throwaway test database, like every other + // account in `global-setup.ts`. Long and mixed so it would also clear the + // registration strength gate. + password: `Cq7!${journey}-${suffix}-Wz4pLx`, + storageStatePath: resolve( + process.cwd(), + `e2e/setup/storageStateCritical-${journey}-${suffix}.json`, + ), + }; +} + +/** Every critical account, for `global-setup.ts` to seed and sign in. */ +export function allCriticalAccounts(): Array< + CriticalAccount & { journey: CriticalJourney } +> { + return CRITICAL_JOURNEYS.flatMap((journey) => + Object.values(CRITICAL_PROJECTS).map((suffix) => ({ + journey, + ...accountFor(journey, suffix), + })), + ); +} + +/** + * The account a journey uses in the project the test is running in. A project + * outside the map has no account on purpose: a critical journey that grows a + * third project has to say which account it reads. + */ +export function criticalAccount( + journey: CriticalJourney, + testInfo: TestInfo, +): CriticalAccount { + const suffix = CRITICAL_PROJECTS[testInfo.project.name as CriticalProject]; + if (!suffix) { + throw new Error( + `[critical-fixture] no ${journey} account for project ${testInfo.project.name}`, + ); + } + return accountFor(journey, suffix); +} + +/** + * A `test.use({ storageState })` value that picks the jar of the running + * project's account. + */ +export function criticalStorageState(journey: CriticalJourney) { + return async ( + {}: object, + use: (path: string) => Promise, + testInfo: TestInfo, + ): Promise => { + await use(criticalAccount(journey, testInfo).storageStatePath); + }; +} + +async function withPool(fn: (pool: pg.Pool) => Promise): Promise { + const url = process.env.DATABASE_URL; + if (!url) throw new Error("[critical-fixture] DATABASE_URL is not set"); + const pool = new pg.Pool({ connectionString: url }); + try { + return await fn(pool); + } finally { + await pool.end(); + } +} + +/** + * Run statements against one critical account, with `$1` bound to its id. + * Fixture hygiene only: what a previous run or a retry left behind. Nothing a + * journey asserts is written here. + */ +export async function resetCriticalAccount( + account: CriticalAccount, + statements: readonly string[], +): Promise { + await withPool(async (pool) => { + const res = await pool.query<{ id: string }>( + "SELECT id FROM users WHERE username = $1", + [account.username], + ); + const id = res.rows[0]?.id; + if (!id) { + throw new Error( + `[critical-fixture] ${account.username} not seeded — global-setup must run first`, + ); + } + for (const sql of statements) await pool.query(sql, [id]); + }); +} + +/** + * Clear the login buckets. The ceiling is five attempts per IP per quarter + * hour, the suite shares one IP, and global setup alone signs in more than + * twenty times; a journey that signs in through the form would otherwise meet + * the fixture's 429 rather than the product. Only the auth surfaces' buckets + * are touched, exactly as global setup does between its own logins. + */ +export async function clearLoginBuckets(): Promise { + await withPool(async (pool) => { + await pool.query(`DELETE FROM rate_limits WHERE key LIKE 'auth:%'`); + }); +} diff --git a/e2e/setup/global-setup.ts b/e2e/setup/global-setup.ts index faf10ceb1..0c38ef96c 100644 --- a/e2e/setup/global-setup.ts +++ b/e2e/setup/global-setup.ts @@ -33,6 +33,7 @@ import { } from "@playwright/test"; import pg from "pg"; +import { allCriticalAccounts } from "./critical-fixture"; import { seedDayDoorsRecord } from "./day-doors-fixture"; import { seedMobileRoutesRecord } from "./mobile-routes-fixture"; @@ -1539,6 +1540,49 @@ export default async function globalSetup(config: FullConfig): Promise { [cuid(), now, E2E_BACKUP_ADMIN.username, E2E_BACKUP_DELEGATE.username], ); + // ── the @critical journeys' accounts ───────────────────────────────── + // + // One per journey and per project; `critical-fixture.ts` says why. Every + // module a journey reaches is switched on (wellbeing and labs are opt-in), + // and the passkeys the sign-in journey registers go with the row's reset, + // so a re-run registers into an empty list rather than beside the last + // run's credential. + for (const account of allCriticalAccounts()) { + await pool.query( + `INSERT INTO users + (id, username, email, password_hash, role, created_at, updated_at, + onboarding_completed_at, onboarding_tour_completed, + module_preferences_json) + VALUES ($1, $2, $3, $4, 'USER', $5, $5, $5, true, $6::jsonb) + ON CONFLICT (username) DO UPDATE SET + email = EXCLUDED.email, + password_hash = EXCLUDED.password_hash, + updated_at = EXCLUDED.updated_at, + onboarding_completed_at = EXCLUDED.onboarding_completed_at, + onboarding_tour_completed = EXCLUDED.onboarding_tour_completed, + module_preferences_json = EXCLUDED.module_preferences_json, + totp_confirmed_at = NULL`, + [ + cuid(), + account.username, + account.email, + await hashPassword(account.password), + now, + JSON.stringify({ + mood: true, + labs: true, + mentalHealth: true, + medications: true, + }), + ], + ); + await pool.query( + `DELETE FROM passkeys + WHERE user_id = (SELECT id FROM users WHERE username = $1)`, + [account.username], + ); + } + // Console (instead of structured logging) is intentional here — // global-setup runs outside the app's logging context, and the // line is useful when debugging a CI failure where the seed didn't @@ -1708,6 +1752,12 @@ export default async function globalSetup(config: FullConfig): Promise { `UPDATE users SET totp_confirmed_at = $2 WHERE username = $1`, [E2E_SETUP_CHILD.username, now], ); + + // The @critical journeys' jars. The sign-in journey signs in through the + // form, so its account gets a jar only for the steps that start signed in. + for (const account of allCriticalAccounts()) { + await capture(account, account.storageStatePath); + } } finally { await pool.end(); } diff --git a/package.json b/package.json index a8f6354bb..2094a014f 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "test:watch": "vitest", "test:integration": "vitest run --config vitest.integration.config.mts", "e2e": "playwright test", + "e2e:critical": "playwright test --grep @critical", "e2e:ui": "playwright test --ui", "openapi:generate": "npx tsx scripts/generate-openapi.ts", "openapi:check": "npx tsx scripts/check-openapi.ts", diff --git a/src/__tests__/e2e-critical-no-api-stub-guard.test.ts b/src/__tests__/e2e-critical-no-api-stub-guard.test.ts new file mode 100644 index 000000000..578effb68 --- /dev/null +++ b/src/__tests__/e2e-critical-no-api-stub-guard.test.ts @@ -0,0 +1,279 @@ +/** + * Structural guard: an `@critical` e2e journey talks to the real server. + * + * The `@critical` tag marks the journeys a release is gated on — sign-in, + * a reading reaching the dashboard, a dose moving compliance, a backup coming + * back. They are only worth that role if the server under them is the one + * that ships. A spec that stubs `/api/*` with `page.route(...)` proves that + * the UI renders a payload somebody typed into the test, which is how a broken + * write path stays green: the mock answers 200 whatever the route does. + * + * So a spec FILE that carries `@critical` anywhere may not intercept the app's + * API, and the rule is per file rather than per test because a route armed in + * a `beforeEach` or a `describe`-level hook covers every test beside it. + * What it catches: + * + * - `.route(` / `.routeFromHAR(` in the spec whose first argument mentions + * `api`, or is not a literal at all (a pattern held in a variable cannot be + * shown to miss `/api`, so it is refused rather than trusted); + * - a helper imported from a relative module whose exported function arms + * such a route (`stubDashboardAnalytics`, `mockMedicationsApi`, …). The + * helper is read, its exported functions are cut apart at each top-level + * `export`, and any that intercepts `/api` may not be imported by name. + * + * Stubbing a third-party host that is unreachable from CI and is not the + * journey's subject stays possible: a literal pattern without `api` passes. + * The spec is expected to say why next to the call. + * + * It is a tripwire with a scanner's limits, not a parser: a route armed + * through a helper that is not imported by name (a default export, a + * re-export chain) is not followed. The synthetic cases at the bottom prove + * each matcher fires; the floor proves the sweep found the tagged tests at + * all. + */ +import { existsSync } from "node:fs"; +import { dirname, join, relative } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + readSourceText, + requireFloor, + walkSourceFiles, +} from "./helpers/source-files"; + +const E2E = join(process.cwd(), "e2e"); + +/** + * The `@critical` tests found in `e2e/*.spec.ts`. Pinned at the real count: + * the set is the release gate, so a journey that loses its tag is a change + * somebody should see here. + */ +const CRITICAL_TEST_FLOOR = 7; + +/** A test whose title literal carries the tag, however the call is spelled. */ +const CRITICAL_TEST = + /\btest(?:\.(?:only|skip|fixme|fail|slow))?\(\s*(["'`])((?:(?!\1)[\s\S])*?@critical(?:(?!\1)[\s\S])*?)\1/g; + +/** A route interception call; the first argument is examined separately. */ +const ROUTE_CALL = /\.route(?:FromHAR)?\(\s*/g; + +/** + * The source without its comment LINES: a line whose first non-blank text is + * `//`, and a block that opens at the start of a line. Deliberately not the + * shared `stripComments`, which opens a block at any `/*` — and route globs + * (`"**\/api/day/**"`, `"https://host/**"`) are full of them, so it would + * swallow the code after the first glob, including any `.route(` there. Prose + * about this rule lives in comment lines, which is what has to go. + */ +function stripCommentLines(source: string): string { + return source + .replace(/^[ \t]*\/\*[\s\S]*?\*\//gm, "") + .replace(/^[ \t]*\/\/.*$/gm, ""); +} + +interface StubFinding { + /** Where the interception lives (the spec, or the helper it imports). */ + where: string; + /** What was found. */ + detail: string; +} + +/** + * The first argument of a call whose open paren ends at `start`: a string, + * template or regex literal is returned whole; anything else is returned up to + * the first comma or closing paren and flagged as not a literal. + */ +function firstArgument( + source: string, + start: number, +): { text: string; literal: boolean } { + const opener = source[start]; + if (opener === '"' || opener === "'" || opener === "`") { + const end = source.indexOf(opener, start + 1); + return { text: source.slice(start, end + 1), literal: true }; + } + if (opener === "/") { + // A regex literal: up to the next unescaped slash. + let i = start + 1; + while (i < source.length && !(source[i] === "/" && source[i - 1] !== "\\")) + i += 1; + return { text: source.slice(start, i + 1), literal: true }; + } + const rest = source.slice(start); + const stop = rest.search(/[,)]/); + return { text: rest.slice(0, stop === -1 ? 80 : stop), literal: false }; +} + +/** Every `/api` (or unprovable) interception in one source text. */ +function apiStubsIn(source: string): string[] { + const code = stripCommentLines(source); + const hits: string[] = []; + for (const m of code.matchAll(ROUTE_CALL)) { + const arg = firstArgument(code, (m.index ?? 0) + m[0].length); + if (!arg.literal) { + hits.push(`route pattern is not a literal: ${arg.text.trim()}`); + } else if (/api/i.test(arg.text)) { + hits.push(`route intercepts the app's API: ${arg.text}`); + } + } + return hits; +} + +/** The names a spec imports from each relative module. */ +function relativeImports( + source: string, +): Array<{ names: string[]; from: string }> { + const code = stripCommentLines(source); + const out: Array<{ names: string[]; from: string }> = []; + for (const m of code.matchAll( + /import\s*(?:type\s*)?\{([^}]*)\}\s*from\s*["'](\.{1,2}\/[^"']+)["']/g, + )) { + const names = m[1] + .split(",") + .map((n) => + n + .trim() + .replace(/^type\s+/, "") + .split(/\s+as\s+/)[0] + .trim(), + ) + .filter(Boolean); + out.push({ names, from: m[2] }); + } + return out; +} + +/** + * Exported functions of a helper that intercept `/api`, by name. The helper is + * cut at each top-level `export`, so each slice is one export's body. + */ +function stubbingExports(helperSource: string): string[] { + const code = stripCommentLines(helperSource); + const starts = [...code.matchAll(/^export\s+/gm)].map((m) => m.index ?? 0); + const names: string[] = []; + starts.forEach((start, i) => { + const slice = code.slice(start, starts[i + 1] ?? code.length); + const name = slice.match( + /^export\s+(?:async\s+)?(?:function\s*\*?\s*|const\s+|let\s+)([A-Za-z0-9_$]+)/, + )?.[1]; + if (name && apiStubsIn(slice).length > 0) names.push(name); + }); + return names; +} + +function resolveHelper(specRel: string, from: string): string | null { + const base = join(E2E, dirname(specRel), from); + for (const candidate of [`${base}.ts`, `${base}/index.ts`, base]) { + if (candidate.endsWith(".ts") && existsSync(candidate)) return candidate; + } + return null; +} + +/** Everything that stubs `/api` in one spec file, its imported helpers included. */ +function findApiStubs( + specRel: string, + source: string, + readHelper: (absolute: string) => string = readSourceText, +): StubFinding[] { + const findings: StubFinding[] = apiStubsIn(source).map((detail) => ({ + where: specRel, + detail, + })); + for (const { names, from } of relativeImports(source)) { + const helper = resolveHelper(specRel, from); + if (!helper) continue; + const stubbing = new Set(stubbingExports(readHelper(helper))); + for (const name of names) { + if (stubbing.has(name)) { + findings.push({ + where: relative(E2E, helper), + detail: `\`${name}\` (imported by ${specRel}) intercepts the app's API`, + }); + } + } + } + return findings; +} + +function specFiles(): string[] { + return walkSourceFiles(E2E, { floor: 100, extensions: [".ts"] }).filter( + (rel) => !rel.includes("/") && rel.endsWith(".spec.ts"), + ); +} + +function criticalTests(): Array<{ file: string; title: string }> { + const tests: Array<{ file: string; title: string }> = []; + for (const file of specFiles()) { + const code = stripCommentLines(readSourceText(join(E2E, file))); + for (const m of code.matchAll(CRITICAL_TEST)) { + tests.push({ file, title: m[2] }); + } + } + return tests; +} + +describe("@critical e2e journeys run against the real API", () => { + it("finds the tagged journeys", () => { + requireFloor("@critical e2e tests", criticalTests(), CRITICAL_TEST_FLOOR); + }); + + it("no file carrying @critical intercepts /api, directly or through a helper", () => { + const files = new Set(criticalTests().map((t) => t.file)); + // A file that mentions the tag outside a test title (a describe title, a + // `grep` option) is held to the same rule. + for (const file of specFiles()) { + if ( + stripCommentLines(readSourceText(join(E2E, file))).includes("@critical") + ) + files.add(file); + } + const offenders = [...files].flatMap((file) => + findApiStubs(file, readSourceText(join(E2E, file))).map( + (f) => `${f.where}: ${f.detail}`, + ), + ); + expect(offenders).toEqual([]); + }); + + describe("the matchers fire", () => { + it("on a glob, a regex and a non-literal pattern", () => { + expect( + apiStubsIn(`await page.route("**/api/measurements*", f);`), + ).toHaveLength(1); + expect( + apiStubsIn(`await page.route(/\\/api\\/analytics(\\?|$)/, f);`), + ).toHaveLength(1); + expect(apiStubsIn(`await context.route(PATTERN, f);`)).toHaveLength(1); + expect( + apiStubsIn(`await page.routeFromHAR("api.har", { update: false });`), + ).toHaveLength(1); + }); + + it("not on a third-party host, a comment, or an unrelated call", () => { + expect( + apiStubsIn(`await page.route("https://tiles.example.org/**", f);`), + ).toEqual([]); + expect(apiStubsIn(`// page.route("**/api/x", f)`)).toEqual([]); + expect(apiStubsIn(`router.push("/api/x");`)).toEqual([]); + }); + + it("on a stubbing helper imported by name, and not on its siblings", () => { + const helper = [ + `export function mockThings(page) {`, + ` void page.route("**/api/things**", (r) => r.fulfill({}));`, + `}`, + `export async function openThing(page) {`, + ` await page.goto("/things");`, + `}`, + ].join("\n"); + expect(stubbingExports(helper)).toEqual(["mockThings"]); + + const read = () => helper; + const uses = `import { mockThings, openThing } from "./medications-wizard-helpers";`; + expect(findApiStubs("x.spec.ts", uses, read)).toHaveLength(1); + const clean = `import { openThing } from "./medications-wizard-helpers";`; + expect(findApiStubs("x.spec.ts", clean, read)).toEqual([]); + }); + }); +});